Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: resolved inconsistent exports in ESM #2047

Merged
merged 4 commits into from
Jun 16, 2022

Conversation

kyranet
Copy link
Contributor

@kyranet kyranet commented Jun 2, 2022

Adds ESM support, for users who prefer using import and libraries using canvas that provides ESM bindings.

Thanks for contributing!

  • Have you updated CHANGELOG.md?

index.mjs Outdated Show resolved Hide resolved
Co-authored-by: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
@LinusU
Copy link
Collaborator

LinusU commented Jun 15, 2022

As far as I can tell, the library already works perfectly fine to import via ESM, why is this needed?

$ echo '{"type":"module"}' > package.json

$ npm add canvas

$ cat index.js
import { Canvas } from 'canvas'

console.log(Canvas)

$ node index.js
[Function: Canvas] {
  _registerFont: [Function: _registerFont],
  _deregisterAllFonts: [Function: _deregisterAllFonts]
}

@kyranet
Copy link
Contributor Author

kyranet commented Jun 15, 2022

Which version of Node.js? I'm trying to import createCanvas as well as loadImage, but they seem to be nested (according to import('canvas')):

[Module: null prototype] {
  Canvas: [Function: Canvas] {
    _registerFont: [Function: _registerFont],
    _deregisterAllFonts: [Function: _deregisterAllFonts]
  },
  CanvasGradient: [Function: CanvasGradient],
  CanvasRenderingContext2D: [Function: CanvasRenderingContext2D],
  Context2d: [Function: CanvasRenderingContext2D],
  default: {
    Canvas: [Function: Canvas] {
      _registerFont: [Function: _registerFont],
      _deregisterAllFonts: [Function: _deregisterAllFonts]
    },
    Context2d: [Function: CanvasRenderingContext2D],
    CanvasRenderingContext2D: [Function: CanvasRenderingContext2D],
    CanvasGradient: [Function: CanvasGradient],
    CanvasPattern: [Function: CanvasPattern],
    Image: [Function: Image] { MODE_IMAGE: 1, MODE_MIME: 2 },
    ImageData: [Function: ImageData],
    PNGStream: [class PNGStream extends Readable],
    PDFStream: [class PDFStream extends Readable],
    JPEGStream: [class JPEGStream extends Readable],
    DOMMatrix: [class DOMMatrix],
    DOMPoint: [class DOMPoint],
    registerFont: [Function: registerFont],
    deregisterAllFonts: [Function: deregisterAllFonts],
    parseFont: [Function (anonymous)],
    createCanvas: [Function: createCanvas],
    createImageData: [Function: createImageData],
    loadImage: [Function: loadImage],
    backends: {
      ImageBackend: [Function: ImageBackend],
      PdfBackend: [Function: PdfBackend],
      SvgBackend: [Function: SvgBackend]
    },
    version: '2.9.1',
    cairoVersion: '1.17.4',
    jpegVersion: '8',
    gifVersion: '5.2.1',
    freetypeVersion: '2.11.1',
    rsvgVersion: '2.54.0'
  }
}

This is in Node.js v16.15.1. On runtime I get this error:

Uncaught:
SyntaxError: Named export 'loadImage' not found. The requested module 'canvas' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'canvas';

@LinusU
Copy link
Collaborator

LinusU commented Jun 15, 2022

Hmm, this is very interesting. I cannot figure out what the difference is between how we export e.g. Canvas (which works) and loadImage (which doesn't) 🤔

https://github.com/Automattic/node-canvas/blob/master/index.js

update: this article might hold some answers:

https://simonplend.com/node-js-now-supports-named-imports-from-commonjs-modules-but-what-does-that-mean/

update2: changing the exports in index.js to the following fixes the problem:

exports.Canvas = Canvas
exports.Context2d = CanvasRenderingContext2D // Legacy/compat export
exports.CanvasRenderingContext2D = CanvasRenderingContext2D
exports.CanvasGradient = bindings.CanvasGradient
exports.CanvasPattern = CanvasPattern
exports.Image = Image
exports.ImageData = bindings.ImageData
exports.PNGStream = PNGStream
exports.PDFStream = PDFStream
exports.JPEGStream = JPEGStream
exports.DOMMatrix = DOMMatrix
exports.DOMPoint = DOMPoint

exports.registerFont = registerFont
exports.deregisterAllFonts = deregisterAllFonts
exports.parseFont = parseFont

exports.createCanvas = createCanvas
exports.createImageData = createImageData
exports.loadImage = loadImage

exports.backends = bindings.Backends

/** Library version. */
exports.version = packageJson.version
/** Cairo version. */
exports.cairoVersion = bindings.cairoVersion
/** jpeglib version. */
exports.jpegVersion = bindings.jpegVersion
/** gif_lib version. */
exports.gifVersion = bindings.gifVersion ? bindings.gifVersion.replace(/[^.\d]/g, '') : undefined
/** freetype version. */
exports.freetypeVersion = bindings.freetypeVersion
/** rsvg version. */
exports.rsvgVersion = bindings.rsvgVersion

@kyranet
Copy link
Contributor Author

kyranet commented Jun 15, 2022

Yeah, that also works, I'll update this PR to do those changes instead.

Edit: Done in fbdeeda!

@kyranet kyranet changed the title feat: add ESM support fix: resolved inconsistent exports in ESM Jun 15, 2022
Copy link
Collaborator

@LinusU LinusU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat 👍

@LinusU
Copy link
Collaborator

LinusU commented Jun 16, 2022

CI test failure seems unrelated...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants