Closed codyhatch closed 7 years ago
@codyhatch Looking at 75265ac, that fix only addresses removal of optionalDependencies
that are specified in the top-level package.json
for your project - it does not look to handle the optionalDependencies
specified by your transitive deps.
As a work-around for the time being, you can add fsevents
to the optionalDependencies
in the package.json
for your application.
I think the correct fix here would be to prune optional dependencies that exist anywhere in the tree, as @palmerj3 pointed out in #74.
@JamieMason thoughts?
Agree @DrewML that you'd have to pluck optionalDependencies
from every package.json
in the dependency graph to then prune it from the shrinkwrap.
I'll think about this some more, but I'm initially hesitant because I'm already not content with how much shrinkpack
has needed to be a bag of patches for upsteam npm shrinkwrap
issues. That said, npm shrinkwrap
is shrinkpack
's source of data, so it's pretty difficult to avoid.
I'll close #74 as a duplicate as there is more discussion in this thread.
According to NPM if you remove optionalDependencies in npm-shrinkwrap.json but they still exist in package.json NPM will install them anyways. I totally disagree with this behavior and will be working on a NPM PR to remove this behavior entirely. But I think you could handle this in shrinkpack.
Certainly removing optionalDeps from top level package.json would be doable but it's not clear to me right now if you will also need to handle this for lower level packages. – @palmerj3
I've pushed a change to feat/75-nested-optional-deps which seems to work well, using the "optional": true
properties found in shrinkwraps generated by newer versions of npm.
The test suite will need updating before it can be released though, as the pruning of optional dependencies results in different files in node_modules (the default behaviour when doing an npm install normally is to include optional dependencies).
If I try something like this on a Mac:
I believe that
fsevents
should be pruned from the rewrittennpm-shrinkwrap.json
file. However, it's not, which means that if I try to install this package on a linux server it will fail, becausefsevents
is still attempting to be installed.This is with node v4.5.0, npm v3.10.7, and shrinkpack v0.17.0.
I believe this is the same issue which was raised in #17, and which should have been fixed in 75265ac. Am I doing something wrong, or is there a regression, or...?
My package.json:
My initial
npm-shrinkwrap.json
: https://gist.github.com/codyhatch/56f559be6ef795f155ac2fe2bdf2ec91My rewritten post-shrinkpack
npm-shrinkwrap.json
: https://gist.github.com/codyhatch/adbaa2d904e1ec5238f5ce861dddd3ba