Automattic / node-canvas

Node canvas is a Cairo backed Canvas implementation for NodeJS.
10.16k stars 1.17k forks source link

node-pre-gyp brings in over 100 new dependencies. #1110

Open dragonfire535 opened 6 years ago

dragonfire535 commented 6 years ago

Issue or Feature

I know this was mentioned partially in #1108, but I feel this should be brought up directly into it's own issue.

Before node-pre-gyp, canvas was a very tiny lib, requiring a mere single dependency. This was actually a large draw to the project for me. Now however, as you can see by this lovely visualization, it is over 100. That's a massive increase in the amount of packages we need to bring in to use canvas all for a prebuild that may not even function as expected or be wanted by the developer. I would be fine with this if it were a small package providing the same functionality, but over 100 deps for a prebuild that only ever runs a single time is a little ridiculous. I'm aware replacing request would fix a lot (mapbox/node-pre-gyp#340), but this would still be a huge dependency even without it, and I'm confused as to why this is necessary at all. Please reconsider bringing such a huge dependency into the project, it's really not good for anyone to be subjected to such dependency hell.

Your Environment

piranna commented 6 years ago

I agree, prebuild and prebuild-install has by far a lot less dependencies since they rely in thenode-gyp` package already included with Node.js instead of having it as a direct dependency (and so, forcing it to download and install it again).

spatialillusions commented 6 years ago

FYI. At the moment the download of prebuilt libraries using node-pre-gyp is broken because of https://github.com/mapbox/node-pre-gyp/issues/359 It looks like it is patched and will be fixed in the next release of node-pre-gyp.

chearon commented 6 years ago

FYI. At the moment the download of prebuilt libraries using node-pre-gyp is broken because of mapbox/node-pre-gyp#359

I'm really surprised that issue is weeks old and there aren't more people affected. node-pre-gyp is currently broken for anyone hosting releases on GitHub! It could be that it's slowly affect more people as their versions of node-pre-gyp upgrade themselves.

More reasons to find something better to replace node-pre-gyp...

spatialillusions commented 6 years ago

What might cause added problems is that for people that has updated to npm 5.8.0, pre-gyp is no longer installed by default. https://github.com/mapbox/node-pre-gyp/issues/362 Looks like it will be fixed, in npm, but will cause problems for people running 5.8.0.

piranna commented 6 years ago

prebuild-install do't have these problems...

zbjornson commented 6 years ago

Version 0.9.0 of node-pre-gyp switched from request (44 dependencies) to needle (5 dependencies), which brings the total number of "nodes" reported by anvaka for node-pre-gyp to 70. An improvement but still sort of a lot...

chearon commented 6 years ago

node-pre-gyp also doesn't work correctly with yarn, I forget if we had a discussion about that already but yarn install canvas@next didn't fallback to build for me yesterday, so we should take another look at prebuild-install or something custom

LinusU commented 6 years ago

I was reading a long blog post about a new project that basically shipped the binaries as part of the package to npm. I thought that they had some really good reasons on why to do that, and they had talked to someone at Npm which thought it was a good idea as well.

I just can't remember the name 😆

edit: here we go: https://www.nearform.com/blog/the-future-of-native-modules-in-node-js/

zbjornson commented 6 years ago

I didn't know that yarn doesn't fall back to build, but there's this yarn issue which prevents using --build-from-source: https://github.com/yarnpkg/yarn/issues/5247

I think I sent that same article to @chearon ... Might have been this one? https://www.nearform.com/blog/the-future-of-native-modules-in-node-js/

With prebuildify instead of downloading a prebuild for your platform at installation time, we simply bundle all prebuilds for all platforms inside a ./prebuilds folder in the node_modules before it is published to npm.

chearon commented 6 years ago

yarnpkg/yarn#5247

I didn't investigate deeply but I assumed it was the same issue as that. --fallback-to-build wasn't passed to node-pre-gyp