Add UMD module support Add Matrix property

[albohlabs]

• Edit export as recommended in typescript templates and lodash and angular typings.
• Add Matrix property

Fix for #564

[coveralls]

Coverage decreased (-0.05%) to 86.531% when pulling 1f2cafe on albohlabs:master into 4770c1e on svgdotjs:master.

[coveralls]

Coverage remained the same at 86.586% when pulling 888680c on albohlabs:master into 4770c1e on svgdotjs:master.

[Fuzzyma]

@rdfriedl can you review this please? :)

[hzrd149]

while it dose not completely fix the bug, it dose highlights a big problem with they typings.
and that is

// while this works
var matrix: svgjs.Matrix = new SVG.Matrix();
// this dose not
var matrix: svgjs.Matrix = new SVG.Matrix(1,0,0,1,0,0);

i think the typings are going to need a little bit more work to fix and update them before they match.

also its partly my fault that this bug is here, when i was reviewing the code i found two typos here and here

[hzrd149]

could you explain what this code dose?
because when i test the typings with this code it just says that the library dose not have a default export.

[albohlabs]

The reason for that is that svg.js does not contain a default export. As a result we need to import the entire module content as an object, which is the corresponding es6 syntax. [1]

could you explain what this code dose?

For clarification:

… anywhere the module keyword was used when declaring an internal module, the namespace keyword can and should be used instead. [2]

The export as namespace makes the svgjs namespace available globally if you use the /// <reference directive. [3]

The rest of the code is doing the same as before.

[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#Description
[2] https://www.typescriptlang.org/docs/handbook/namespaces.html
[3] microsoft/TypeScript#7125

[hzrd149]

i did some testing with importing the library and it looks like

import SVG from 'svg.js'

and

import * as SVG from 'svg.js'

returns the default SVG() function.
Its also possible to do

import {Element} from 'svg.js'

with that in mind I tested the old typings file and the one here and this is what i got.

The original
this worked fine

import SVG from 'svg.js'
var draw: svgjs.Doc = new SVG.Doc();

this dose not, because the original typings only exports svgjs.Library

import {Doc} from 'svg.js'
var draw: svgjs.Doc = new Doc();

this dose not work.
it makes SVG a object with the default property set to the default SVG() function

import * as SVG from 'svg.js'
var draw: svgjs.Doc = new SVG.Doc();

With your version
this dose not work, it says that there is no default export

import SVG from 'svg.js'
var draw: svgjs.Doc = new SVG.Doc();

but this works fine.
except for the svgjs.Doc, it says that Namespace 'svgjs' has no exported member 'Doc'

import {Doc} from 'svg.js'
var draw: svgjs.Doc = new Doc();

also this works.
but has the same problem with the svgjs namespace

import * as SVG from 'svg.js'
var draw: svgjs.Doc = new Doc();

Conclusion
it looks like your version works a little better then the original.
we would just have to fix the default export and the namespace error.

[albohlabs]

Great. 🍻

we would just have to fix the default export and the namespace error.

Like you already said in the comment bellow. I will try to do some work on this issue.

[hzrd149]

this makes sense though i think it highlights an even bigger problem with the typings.

when i wrote the typings i forgot to supply the arguments to the methods on the library
what i mean is, this works

var matrix: svgjs.Matrix = new SVG.Matrix();

but this dose not

var element: svgjs.Element = new SVG.Element();
var matrix: svgjs.Matrix = new SVG.Matrix(element);

this is a mistake on my end since i set the Matrix as a function on the Library and not a property that holds a reference to the Matrix class
a few examples, here and here
but i remembered to include in other places, like here

of course this means that we would have to run over the whole library and fix the parts like this

interface Library { Number(): void; }

and change then to something like this

interface Library { Number: Number; }

and then fix this

export interface Number{
    // change this
    (value: any, unit?: any): Number;
    // to this, that way SVG.Number can only be called with the "new"
    new (value: any, unit?: any): Number;
}

so while this dose fix it for the Matrix class i think we will end up having to go over the entire file and fix each class

[albohlabs]

This is a good starting point to fix the typing.

[hzrd149]

I had a lot of free time on my hands so i decided to fix this bug.
If you want to check it out, its over here

Im not an expert in using git or github so im not sure how we would merge my code into this PR.
any ideas?

[albohlabs]

Awesome.

I could cherry pick your commits and include them into the PR, if this is ok for you?

[hzrd149]

Go right ahead, it's better then me creating a whole new PR

[Fuzzyma]

You could also create a PR for his PR by forking his fork and merge it there then merge it here. However: Cherrypicking might be faster ^^

[albohlabs]

Nice try, @Fuzzyma 😝

The commits are now included into the PR. There are no errors when importing and using the Matrix property of the SVG object.
I think we are ready to merge this …

[coveralls]

Coverage increased (+0.05%) to 86.586% when pulling 8559b15 on albohlabs:master into 422b84c on svgdotjs:master.

[hzrd149]

there is still one problem left to fix.
this code works fine

import * as SVG from 'svg.js';
var draw: svgjs.Doc = new SVG.Doc();
document.body.appendChild(draw.node);

but this dose not work anymore

import SVG from 'svg.js';
var draw: svgjs.Doc = new SVG.Doc();
document.body.appendChild(draw.node);

it says that svg.js has no default export...
iv spent a lot of time on this and still can figure out to make it work, any ideas?

[albohlabs]

This is a core design decision. You can see the discussion and conclusions in this issue.
To solve this we could define a default export in the library (and typings) itself or change the import syntax like recommended in this issue.

[albohlabs]

Here is a documentation change which also address our case.

[hzrd149]

oh i see, i guess typescript modules dont work exactly like ES6 modules 😄
In that case everything looks good

@Fuzzyma can you merge this?

[Fuzzyma]

sure thing. Thanks for your work!