Add typescript definitions

[zbjornson]

Thanks for contributing!

• Have you updated CHANGELOG.md?

I'm not a typescript expert, so if anyone sees any better ways to do things, please speak up! :) @varHarrie @adamjking3

Note that typescript between 2.9ish and 3.1ish renamed Canvas2DContextAttributes to CanvasRenderingContext2DSettings. People using older typescript won't have correct definitions.

Fixes #1303

[rauchg]

This would be really nice!

[varHarrie]

It seems good, and we should also specify types in package.json.

[krzysztof-miemiec]

After copy-pasting it into my node project, I see that the typings depend on lib.dom.d.ts typings, and "lib": ["dom"] has to be present in tsconfig.json file - I think that it's worth mentioning in README. I also got the following TypeScript errors:

Error:(130, 5) TS2661: Cannot export 'CanvasGradient'. Only local declarations can be exported from a module.
Error:(131, 5) TS2661: Cannot export 'CanvasPattern'. Only local declarations can be exported from a module.
Error:(140, 5) TS2416: Property 'src' in type 'Image' is not assignable to the same property in base type 'HTMLImageElement'.
  Type 'string | Buffer' is not assignable to type 'string'.
    Type 'Buffer' is not assignable to type 'string'.
Error:(149, 15) TS2702: 'Image' only refers to a type, but is being used as a namespace here.
Error:(149, 34) TS2702: 'Image' only refers to a type, but is being used as a namespace here.
Error:(216, 12) TS2661: Cannot export 'DOMMatrix'. Only local declarations can be exported from a module.
Error:(216, 23) TS2661: Cannot export 'DOMPoint'. Only local declarations can be exported from a module.
Error:(218, 12) TS2661: Cannot export 'ImageData'. Only local declarations can be exported from a module.

I think that reexports can be changed to something like: export const DOMMatrix: DOMMatrix;, however I'm not sure what to do with HTMLImageElement error.

Edit:
Workarounds for all Image errors look like this 😉

  class IMG extends HTMLImageElement {
    src: any;
  }
  
  type ImageMode = number & { __imageMode: true }

  export class Image extends IMG {
    /** Track image data */
    static readonly MODE_IMAGE: ImageMode;
    /** Track MIME data */
    static readonly MODE_MIME: ImageMode;

    src: string | Buffer;
    /**
     * Applies to JPEG images drawn to PDF canvases only. Setting
     * `img.dataMode = Image.MODE_MIME` or `Image.MODE_MIME|Image.MODE_IMAGE`
     * enables MIME data tracking of images. When MIME data is tracked, PDF
     * canvases can embed JPEGs directly into the output, rather than
     * re-encoding into PNG. This can drastically reduce filesize and speed up
     * rendering.
     */
    dataMode: ImageMode;
  }

[krzysztof-miemiec]

Oh, and when I started to use your typings, I saw the need to export NodeCanvasRenderingContext2D from them. I think it's a good practice to export all interfaces that can be returned by functions, so that library user can do something like this:

class MyCanvasWrapper {
  private readonly canvas: Canvas;
  private readonly context: NodeCanvasRenderingContext2D;

  constructor(width: number, height: number) {
    this.canvas = createCanvas(width, height);
    this.context = this.canvas.getContext('2d');
  }
}

[LinusU]

I see that the typings depend on lib.dom.d.ts typings, and "lib": ["dom"] has to be present in tsconfig.json file

I think that this can be fixed by having /// <reference lib="dom" /> at the top of the file

[j]

Status of this?

[zbjornson]

Got busy working on bugfixes -- will update this this weekend.

[zbjornson]

Ready for another review/testing.

Used the instructions in https://github.com/Microsoft/dtslint#add-types-for-a-library-not-on-definitelytyped for setup.

[bmamouri]

@zbjornson Thank you for the great work. Wondering when this will be merged?

[chearon]

Looks good to me!

[LinusU]

Looks great!

[zbjornson]

Just tested this in a module where I'm using node-canvas. I forgot the PNG constants, drawImage(Canvas.Canvas, ...) specialization and canvas.width/height, but I just fixed those and everything seems to work well.

https://github.com/Automattic/node-canvas/pull/1330/files#diff-88bec6beae84369c0376b56b3bb88fe1R200 this is sort of ugly but I don't see a way around it aside from providing all of our own Context2D types. That might not be a bad idea since we don't have full support yet for the entire HTML Canvas API (see https://github.com/Automattic/node-canvas/wiki/Compatibility-Status).

What do folks think we should do?

1. Use this PR as-is and have ~95% accurate types. The types will claim that ctx.direction, ctx.filter, scrollPathIntoView() and a handful of other properties/methods work, but they don't. Some of them never will (like scrollPathIntoView()).
2. Copy from lib.dom.d.ts into this library so that we can remove the things we don't provide and make them ~100% accurate?

[LinusU]

Hmm, seems like OffscreenCanvas gives back a normal CanvasRenderingContext2D when calling getContext(), so I wonder how browsers handle calling scrollPathIntoView() from a web worker 🤔

I think I'm leaning towards option 1, but I could be convinced either way ☺️

[icfantv]

Hey guys, anything I can do to get the ball rolling on this again? Any more updates to the typings that need to happen? Happy to help. Cheers.

[playground]

Will this be released soon?

[zbjornson]

I'll finish up the last set of changes this weekend, sorry for the delay.

[1999]

Wow, nice work @zbjornson! I didn't notice that when I raised a PR for DefinitelyTyped repo (DefinitelyTyped/DefinitelyTyped#33555), so I will close the PR and wait for this one to get merged.

[septs]

quality and patternQuality share typing definition.

type Quality = 'fast' | 'good' | 'best' | 'nearest' | 'bilinear';

quality: Quality
patternQuality: Quality

[demurgos]

Would it be possible to publish this PR and iterate? It's been pending for some time now.

[zbjornson]

grumble a dependency of dtslint added an async function, which breaks our CI that runs with node 6. Thinking of better workarounds, but if I can't think of one today I'll remove dtslint (sadly; it's been critical to testing this) and merge.

[zbjornson]

There we go.

@LinusU since you have to do releases anyway, I'll let you merge. I just changed .travis.yml to only run dtslint with Node v10.

[septs]

https://github.com/Automattic/node-canvas/pull/1330/files#diff-c472f8ca9fb190272d5098f1ccd7bd41R32

requires definition pngStream , jpegStream , pdfStream method typing

[zbjornson]

Ahh I thought you were asking for those classes to be defined, now I see you meant the method aliases. Those method aliases are present for backwards compatibility; we standardized on Node.js core's idiom of create____ in #1152. I'd rather not type them since they're not otherwise documented, but can add them if you think it's important.

[septs]

I think it should be backwards compatible.
Maybe you can learn how to React.

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/e1f9647734bd990f49b0aca0f44acd4d55fe1e93/types/react/index.d.ts#L456-L462

[LinusU]

Nice! I'm out and about at the moment but will try to merge and release this asap!

Feel free to address my Yarn comment, otherwise I'll fix it myself before merging 👍

[playground]

@zbjornson will this be ready soon?

[zbjornson]

It's ready. @LinusU I forgot to post that I addressed your comment.

[playground]

Thanks @zbjornson, can't wait, I'm currently blocked by this :-) as I'm porting existing codebase to Typescript.

[LinusU]

Awesome work as always @zbjornson ❤️

[playground]

@zbjornson @LinusU I'm getting error with tsc

ERROR in ./node_modules/canvas/build/Release/canvas.node 1:0
Module parse failed: Unexpected character '�' (1:0)
You may need an appropriate loader to handle this file type.
(Source code omitted for this binary file)

Actually, still can't get it to transpile, webpack is still complaining.

[icfantv]

@zbjornson @LinusU hey guys, when do you think you'll be able to cut a new release to NPM? Thanks.

[LinusU]

was hoping to get #1255 in after @zbjornson have taken a look at it, but otherwise we can ship that later...

Will try to cut a release tonight or tomorrow, regardless of if it's merged or not ☺️

[zbjornson]

Good plan; I'll try to get to that tonight, but lots of people are waiting on typescript defs, so no need to delay it.

@playground that means something (usually webpack) is trying to load the .node binary as a JS file. If you search for "webpack native module" you should find some advice. One option I've seen people use is https://github.com/webpack-contrib/node-loader, or just not use webpack for Node.js/server-side.