Update sylvester typings to be a cjs module

[connor4312]

Please fill in this template.

• Make your PR against the master branch.
• Use a meaningful title for the pull request. Include the name of the package modified.
• Test the change in your own code. (Compile and run.)
• Follow the advice from the readme.
• Avoid common mistakes.
• Run tsc without errors.
• Run npm run lint package-name if a tslint.json is present.

If changing an existing definition:

• Provide a URL to documentation or source code which provides context for the suggested changes: (see a bit below)
• Increase the version number in the header if appropriate.
• If you are making substantial changes, consider adding a tslint.json containing { "extends": "../tslint.json" }.

Sylvester is exported as a CommonJS module and, while it declares some globals, there are requests to remove them and most usage in TypeScript will prefer to use it as a CommonJS module. I did not see a way to provide both global and module exports in a typings file, so I favored converting it to a CommonJS typing and removing the global exports.

I've also updated it to the recommended modern linting rules and best practices.

[dt-bot]

sylvester/index.d.ts

to author (@StephaneAlie). Could you review this PR?
👍 or 👎?

Checklist

• pass the Travis CI test?

[ghost]

As-is this PR would break compilation for people using globals.
You can fix it like so:

// No `declare namespace sylvester` wrapper

export class Vector { ... }
export class Matrix { ... }
export class Line { ... }
export class Plane { ... }

// You probably meant "namespace" and not "interface".
export namespace Sylvester {
    // `require("sylvester").Sylvester.version` doesn't work for me, but `require("sylvester").Sylvester.approxPrecision` does.
    export const precision: number;
    export const approxPrecision: number;
}

// Re-export as globals.
import * as s from "sylvester";

declare global {
    const Vector: typeof s.Vector;
    type Vector = s.Vector;
    const Matrix: typeof s.Matrix;
    type Matrix = s.Matrix;
    const Line: typeof s.Line;
    type Line = s.Line;
    const Plane: typeof s.Plane;
    type Plane = s.Plane;
    const Sylvester: typeof s.Sylvester;

    // Assuming these are global-only functions
    declare function $V(elements: Vector|Array<number>): Vector;
    ...
    declare function $P(anchor: Array<number>|Vector, v1: Array<number>|Vector, v2: Array<number>|Vector): Plane;
}

Then have both sylvester-tests.ts and a sylvester-global-tests.ts which tests the globals ($V through $P, and tests that Vector is available globally.)
Also, since you're changing the whole thing, please add a tslint.json containing { "extends": "../tslint.json" }.

[connor4312]

Thanks for the thorough review, I've updated the typings 😄

[ghost]

Leaving the rest for @StephaneAlie

[ghost]

Can you verify that this exists?

[ghost]

Reduce indentation (no longer nested)

[StephaneAlie]

That looks a lot cleaner than what I had, thanks for the update!

[DanielRosenwasser]

Any reason you need this?

[ghost]

@connor4312 Please fix the merge conflicts.

[mhegazy]

@connor4312 the test file name needs to be changed to test\global.ts

[zhengbli]

@connor4312 can you update according to the reviews? Thank you!

[jramsay]

@connor4312: if you can take care of the remaining changes from code review comments we'll be able to merge this PR. Thanks!

[yuit]

@connor4312 Thanks for your contribution. This PR has unaddressed comments and cannot be merged in at the time being. For housekeeping purposes we are closing stale PRs. If you'd still like to merge this code in, please open a new PR that has been merged and rebased with the master branch.