search

oclif / plugin-warn-if-update-available

warn user if a new CLI version is available
MIT License
38 stars 13 forks source link

Publishing the shrinkwrap makes npm unnecessarily install the dev dependencies #553

Closed dominykas closed 7 months ago

dominykas commented 7 months ago

Describe the bug

As of 3.0.13, the dev dependencies of @oclif/plugin-warn-if-update-available end up in the end user's node_modules

To Reproduce

devel/experiments/tmp
❯ npm i @oclif/plugin-warn-if-update-available

added 1057 packages in 4s

14 packages are looking for funding
  run `npm fund` for details

devel/experiments/tmp via  v20.11.1 took 4s
❯ npm ls mocha
tmp@ /Users/Dominykas_Blyze/devel/experiments/tmp
└─┬ @oclif/plugin-warn-if-update-available@3.0.14
  └── mocha@10.3.0 extraneous

devel/experiments/tmp via  v20.11.1
❯ rm -rf node_modules

devel/experiments/tmp via  v20.11.1 took 4s
❯ npm i @oclif/plugin-warn-if-update-available@3.0.12

added 107 packages, and audited 108 packages in 962ms

18 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

devel/experiments/tmp via  v20.11.1
❯ npm ls mocha
tmp@ /Users/Dominykas_Blyze/devel/experiments/tmp
└── (empty)

Expected behavior

Dev dependencies should not be installed

Screenshots

n/a

Environment (please complete the following information):

Additional context

FWIW, npm has never done a good job respecting the shrinkwraps included as part of dependencies - it does not work consistently or reliably (it can be ignored at times, pulling in latest dependencies, rather than what is pinned, etc).

Using a shrinkwrap also creates problems in deduplication, e.g. @oclif/plugin-plugins has oclif@4.5.2, but latest is oclif@4.5.4 and so you end up with two versions of oclif in your dependency tree.

mdonnalley commented 7 months ago

Thanks @dominykas for the issue.

For context, we started publishing shrinkwraps so that the forthcoming version of plugin-plugins (PR) can install plugins with locked dependencies.

Not sure what we'll do about your use case but we'd love any ideas that you might have

mdonnalley commented 7 months ago

@dominykas After thinking this over a bit more, I don't think this is something that we're going to address. We need to ship shrinkwraps for our plugins so that plugins install will install the plugins with the locked versions. So if you or others need to get rid of the devDependencies during your release, you can run npm prune --omit=dev.

dominykas commented 6 months ago

npm prune --omit=dev also removes our own dev dependencies. This is not an issue for the final production release - we were already doing it. It is however a problem for e.g. various dependency vulnerability scanning tools (e.g. snyk), because it starts complaining about old/vulnerable versions of mocha in our dependency tree if we don't prune, and if we do prune - we lose insights into our direct dev dependencies.

And like I said, npm always had issues with shrinkwraps - it's riddled with bugs (and these are just the confirmed and open ones) and does not actually give you the guarantees you think you're getting in reality. It is ofc a confirmed bug on npm side that these dev dependencies are installed at all, but that does not change the fact that the results are unexpected on the end user side.

Edit: ironically, when you run npm prune, you can actually end up with more packages than you had at the start 😁 but I'm not going to bother filing an issue for that...

dominykas commented 6 months ago

Yet another case-in-point: an npm sub-dependency (tar) has a vulnerability. Installing just the base oclif gives you a version that is a minor behind, but depending on which plugins you have installed - you could have three different versions, all of them vulnerable.

Surely there must be a better way?