Closed yiochen closed 6 years ago
Shouldn't having it in dependencies
ensure node-pre-gyp
is present? It sounds like bundledDependencies
copies dependencies into the tarball before publishing, but we shouldn't have to force people to have multiple copies of node-pre-gyp
.
Yeah, Mapbox doesn't recommend it anymore:
Note: in the past we recommended putting node-pre-gyp in the bundledDependencies, but we no longer recommend this. In the past there were npm bugs (with node versions 0.10.x) that could lead to node-pre-gyp not being available at the right time during install (unless we bundled). This should no longer be the case.
https://github.com/mapbox/node-pre-gyp/blob/master/README.md
I am also confused. This seems to be an issue with npm ci
.
I managed to work around it by adding node-pre-gyp
to my package.json
Yeah... seems like a bug in npm
. Feel free to re-open if you find otherwise.
I think that npm
is working as intended and this is actually an issue with canvas-prebuilt
. It has been fixed too for over a year, but only in v2 alphas (#13). I wonder if it's possible to backport that fix to the latest
tag too?
The isssue is that npm
dedupes node-pre-gyp
to top-level but the install script of canvas-prebuilt
assumes that it's found in its own node_modules
folder.
I.e. assumed structure:
project-root
└ node_modules
└ canvas-prebuilt
└ node_modules
└ node-pre-gyp
Actual structure:
project-root
└ node_modules
├ canvas-prebuilt
└ node-pre-gyp
Turns out the install script doesn't work at all on Windows native command line due to it having forward slashed and no quotes. This was of course the original reason for #13, it just also happened to fix the missing node-pre-gyp
issue.
@chearon could this fix be backported to latest
package too (i.e. version 1.x)?
Yeah sorry, I'll try to do that soon, someone else just asked for it in #51.
Can you publish that 1.6.12 version, please. The npm ci
still doesn't work.
Sorry I missed that, it's been difficult supporting 1.x which is an old/broken build and the 2.x series and build.
I pushed the commit @Pumpuli mentioned to the 1.x build branch for the next release. As for a 1.6.12, version of canvas-prebuilt
, I screwed that up a while back and unpublished it because it accidentally included canvas
as a dependency 🤦♂️ so should I:
canvas-prebuilt@1.6.13
that's really canvas@1.6.12
canvas@1.6.13
noop release so I can create a canvas-prebuilt@1.6.13
?I'm not sure which is worse
Thank you for your answer, I think the latter is better, but it is just my opinion. I can use 2.x if it is working there, but I see it as alpha version. Is it stable now?
There's one known regression in both 2.x and 1.6.12 (https://github.com/Automattic/node-canvas/issues/1249). It has a PR to fix it that needs to be reviewed. Otherwise 2.x is stable.
Step to reproduce:
throws error:
From this, it seems like we need to list
node-pre-gyp
as bundledDependencies in package.jsonnode -v 10.3.0 npm -v 6.1.0