Fabric.js: Migrate optionalDependencies to peerDependencies

[ramijarrar]

Following the discussion in #2775 - node-only packages are now defined as optional dependencies, providing support for browser-only usage via the --no-optional flag.

While this is definitely a step forward, the lack of support for specifying this flag on a package-by-package basis or even at the project level config (see https://github.com/npm/npm/issues/14185) makes this solution inadequate in a number of ways:

1) Prevents the use of optional dependencies within *any* context (e.g if we want to use the server-side version of a different package taking a similar approach)

2) Breaks the npm install command for programs consuming Node applications (e.g hosting platforms, build tools, etc.)

While the original suggestion to publish multiple packages stands, we should be able to circumvent these concerns by moving to peer dependencies (an approach taken by other popular packages such as eslint-config-airbnb)

We would then use npm install --save fabric for browser-only and npm install --save fabric canvas jsdom xmldom when we want Node support.

(borrows from parallel issue @ https://github.com/hydrabolt/discord.js/issues/787)

[asturur]

what happens to current node projects that do npm install fabric?

[ramijarrar]

Existing projects can continue using older versions without any problems.

Once they do decide to upgrade, they will get a warning about the peer dependencies (which they can choose to ignore for browser-only)

[ramijarrar]

Okay so I've been experimenting with the peer dependencies option in our own fork - and it has created a new issue with shrinkwrap (tracked in https://github.com/npm/npm/issues/12909)

It really doesn't look like we can solve this properly without splitting the dependencies off to a separate package.

[StephanBijzitter]

One thing to note is that doing this will break compatibility in all cases, and as such will need a new major version.

[asturur]

there is a npm package called canvas-prebuilt, is node canvas ready make for macosx, linux, windows.
I would switch to that and get rid of half of the problems for installation.

in current master we switched to node4+ and jsdom 9+, so the compilation error should be less and less

[asturur]

i opened a pull request to JSDOM to enable canvas-prebuilt.
That should solve most of our issues installing fabric.js

If you want to go there and share your thoughts:
https://github.com/tmpvar/jsdom/pull/1754

[asturur]

i switched to canvas-prebuilt.

Full install and test run dropped to less than 2 minutes in travis.

no lib requirements, no gcc requirements.

[cogell]

Heres a PR for this: https://github.com/kangax/fabric.js/pull/3788

[asturur]

@cogell what's the problem with canvas-prebuilt has of now? do you have any difficulties installing it?

[cogell]

@asturur we had issues on mac sierra and on our jenkins running centos so we made this fork. I don't know why the canvas-prebuilt didn't work for those instances.

[tunderdomb]

I think this is related to this issue:

Installing 1.7.9 fails on the latest node because of an outdated jsdom dependency.

npm view fabric dependencies shows that the currently published package.json has an explicit dependencies field with "jsdom": "3.x.x" in it. However I couldn't find a relevant branch in this repo where it looks like that.

The problem is that [email protected] pulls contextify which fails on >=node7: https://github.com/brianmcd/contextify/issues/205.

[asturur]

branch 1.x uses jsdom 3.x and canvas
branch master use latest jsdom and canvas-prebuilt. new master branch still have not a version number nor a release.
i need to finish some small text work and then i can merge it and release as 2.0alpha, but many api will be changed until i release 2.0

[asturur]

@cogell talking with author of canvas-prebuilt looks like that the linux version had a problem on some bundled lib.

[smnbbrv]

@asturur what about contextify errors on newer node versions? I am forced to use > v7 and the error message is really annoying

gyp ERR! command "/usr/local/Cellar/node/8.1.3/bin/node" "/Users/***/node_modules/.bin/node-gyp" "rebuild"
gyp ERR! cwd /***
gyp ERR! node -v v8.1.3
gyp ERR! node-gyp -v v3.6.2
gyp ERR! not ok 
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: [email protected] (node_modules/contextify):
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: [email protected] install: `node-gyp rebuild`
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: Exit status 1

[asturur]

the version 2 uses newer jsdom and there is no that error.

version 1.x support up to node 6. no higher

[aggrosoft]

@asturur sorry to resurrect this topic - you said that the master will be using canvas-prebuilt now. But from what I see here: https://github.com/fabricjs/fabric.js/blob/master/package.json#L63 this is still on normal canvas?

[asturur]

Canvas itself now comes with prebuilt, is way easier to install. we can still use the canvas-prebuilt packages.

Switching to peerDeps means killing any existing node setup that is working ( because canvas may not be in the deps list ) and also does not save you from figuring out how to install it if you need it.

I opened a repo called fabricjs-webpack just to show an installation where out of the box node canvas fails installing but everything else looks good

[aggrosoft]

Maybe this is not the right place to discuss, but still I have to ask. I tried installing fabric via npm on host that does not have the needed packages to compile the canvas package. Install does not fail, but it won't install canvas.

> node-gyp rebuild

./util/has_lib.sh: 31: ./util/has_lib.sh: pkg-config: not found
gyp: Call to './util/has_lib.sh freetype' returned exit status 0 while in binding.gyp. while trying to load binding.gyp
gyp ERR! configure error

This leads to the following error when running my script:

/.../node_modules/fabric/dist/fabric.js:6976
      ctx.imageSmoothingEnabled = ctx.imageSmoothingEnabled || ctx.webkitImageSmoothingEnabled
                                      ^

TypeError: Cannot read property 'imageSmoothingEnabled' of null

which is because canvas was not installed. When I add canvas-prebuilt to my dependencies and set

fabric.canvasModule = 'canvas-prebuilt';

it still fails with the same error. canvas-prebuilt installs just fine. So is there any documentation or something like that which shows how to properly use fabricjs on node with canvas-prebuilt?

[asturur]

so JSDOM should see canvas-prebuilt. And maybe our internal setting is no more usefull. I wonder what is going on. What system are you on?

[aggrosoft]

I'm on:

Linux 4.9.0-8-amd64 #1 SMP Debian 4.9.110-3+deb9u5 (2018-09-30) x86_64 GNU/Linux

I just installed the node-canvas dependencies, npm install then compiled everything - this works. But this is quite a hassle as I have to deploy the application to different systems. It looks like jsdom ignores my canvas-prebuilt when I add it, I just added it to my dependencies like this:

"dependencies": {
    "fabric": "^2.4.2-b",
    "canvas-prebuilt": "^1.6.11"
  }

And as mentioned before was using the extra fabric.canvasModule flag. I also suspect this is something with jsdom, but I can't tell for sure. Isn't this expected to work?

[asturur]

it may be that when we switched to full Jsdom leverage, we lost the ability to choose which to use. I committed a patch to jsdom in order to either use canvas or canvas prebuilt depending on what is available. I wonder if an errored canvas will eventually result as available to JSDOM but not being functional

[aggrosoft]

Ok, so since canvas-2 is officially out and jsdom is planning to migrate to it this issue is not relevant anymore. Canvas-2 is using a prebuilt version anyways. What are the plans for fabricjs support for canvas-2 ? Is this sheduled for 3.x ?

[asturur]

we need to do a couple of things.
Deprecate stuff we want to kill in 4.0
rename a couple of bad named variables
remove the things we deprecated in 2.0
...
I think there was some proposal around, but quick stuff.