search

zkat / npx

execute npm package binaries (moved)
https://github.com/npm/npx
Other
2.63k stars 105 forks source link

npx unexpectedly uses npm from dependencies #127

Open boennemann opened 7 years ago

boennemann commented 7 years ago

I know that npx is designed to use binaries from the local node_modules/.bin folder. Today I stumbled across something where this behavior leaves the user with totally unexpected (or dangerous even) results.

For example restify accidentally added a dev tool called unleash as a dependency and published that to the registry. Version 6.2.0 of restify now had npm@2 in it's dependency tree, via unleash. (Fixed via https://github.com/restify/node-restify/pull/1522)

This caused every script executed by npx that uses npm to use npm@2. (Same is true for npm run apparently: https://github.com/npm/npm/issues/18874)

For example calling npx npm-check -u ended up trying to install packages with npm@2, which failed. The same happened for greenkeeper-lockfile if used through npx: This command npx -p greenkeeper-lockfile@1 greenkeeper-lockfile-update picked up npm@2 under the hood, and started failing.

I imagine similar, or worse, accidents will happen if somehow npm.im/node starts showing up in the dependency tree.

I know this is kind of a hard problem to solve, by the design of npx, but maybe there should be an additional "opt-in". Would it be possible to change npx so that it will pick up only top-level dependencies' binaries, or the ones passed via -p cli flag? (At least for npm and node).

zkat commented 7 years ago

Oh interesting.

I'd say this is working exactly as designed: npx is meant to allow you to work within the local environment.

I'd argue that this isn't an issue with npx itself, as much as it is an issue with the way npm itself links binaries: the fact that your toplevel .bin can leak dependencies' binaries unexpectedly is certainly something that concerns me about npm itself, and if it's not npm breaking, it would absolutely be an issue with your run-scripts: "scripts": { "update-deps": "npm-check -u" }, "devDepencencies": { "npm-check": "*" } would put you in the same place.

In fact, I know other folks have brought up this issue with me before. I'm gonna /cc @iarna 'cause we've definitely talked about dealing with the bin linking in npm, and this seems like, if it's a breaking change, would be a welcome one to have in npm@6.