ncc tries to bundle canvas which is optional

[NathanielHill]

I'm trying to build a lambda using jsdom. Everything works fine locally, unfortunately when ncc tries to build it wants to bundle canvas because it gets tripped up by the following code:

https://github.com/jsdom/jsdom/blob/16d3913eea0360c7757e75f7e266c3873c85b7dd/lib/jsdom/utils.js#L163

exports.Canvas = null;
let canvasInstalled = false;
try {
  require.resolve("canvas");
  canvasInstalled = true;
} catch (e) {
  // canvas is not installed
}
if (canvasInstalled) {
  const Canvas = require("canvas");
  if (typeof Canvas.createCanvas === "function") {
    // In browserify, the require will succeed but return an empty object
    exports.Canvas = Canvas;
  }
}

Unfortunately, puppeteer-core, chrome-aws-lambda, jsdom, and canvas cannot all fit under 50MB.
I am not using any <canvas> tags, so this is unnecessary.

Any thoughts on a workaround besides forking jsdom?

@guybedford

[rauchg]

In this case, do you have a canvas dep? Or is ncc erroring?

[NathanielHill]

No dependency. jsdom will conditionally use it if it's installed. Unfortunately, whatever static analysis ncc is doing gets caught up on the previous lines and tries to bundle canvas despite it being unnecessary and unspecified in package.json

[NathanielHill]

@rauchg it's seeing the require('canvas') even though it's in a conditional it will never reach

[NathanielHill]

I suspect this could more easily be fixed by jsdom, and I can open an issue there. But at the same time it's not clear that it's really their fault.

[NathanielHill]

I haven't looked at ncc at all yet, but if the argument to require() is a variable that's unknown at bundling time... ncc wouldn't be able to pick up on that I imagine?

Perhaps

canvasInstalled = 'canvas'

...

If (canvasInstalled) {
  require(canvasInstalled)
}

Would be a workaround?

[guybedford]

@NathanielHill did you try setting canvas as an external here? ncc build app.js -e canvas?

[NathanielHill]

@guybedford

No I haven't, this would probably fix it, but it's being deployed on Zeit. Is there a way to pass ncc options through now.json?

[guybedford]

I'm not sure there is. That sounds like it would be a worthwhile feature.

[NathanielHill]

@guybedford Yes :pray: because otherwise I don't know how this is solvable. jsdom has already said they won't make any changes, which is not entirely unreasonable.

[NathanielHill]

Seems like dynamic imports is an ongoing problem, and the most pragmatic thing might be simply having an escape hatch to pass options to ncc?

[styfle]

This appears to be fixed in the latest ncc.

Steps to reproduce

yarn init -y
yarn add jsdom
echo "let dom = require('jsdom')" > index.js
ncc build index.js

du -sh dist # 4.6M

rm -rf dist # clear output

ncc build index.js -e canvas

du -sh dist # 4.6M

The size didn't change with the external flag and since canvas is 90M, I don't think it is included.

Also, I checked the jsdom dependency tree and canvas is not included. So I'm not sure how this caused a problem in the first place.

@NathanielHill Can you confirm this is fixed for you?

[NathanielHill]

unable to reproduce this now