Skip to content
This repository has been archived by the owner. It is now read-only.

`npm v5`: `npm install` auto-prunes? #16853

Open
ljharb opened this issue Jun 1, 2017 · 50 comments
Open

`npm v5`: `npm install` auto-prunes? #16853

ljharb opened this issue Jun 1, 2017 · 50 comments
Labels

Comments

@ljharb
Copy link
Contributor

@ljharb ljharb commented Jun 1, 2017

In enzyme, we need to support multiple react versions, so we don't depend directly on react. Instead, we use the linked run-scripts in CI to install the version under test with --no-save, to avoid modifying package.json, and then run npm install.

It appears that when something is installed to disk, but not in package.json, npm install now erases it. This doesn't appear in any documentation I've seen.

Is this intentional? If so, how can I bypass it for these run-scripts' npm install command?

@ljharb

This comment has been minimized.

Copy link
Contributor Author

@ljharb ljharb commented Jun 1, 2017

ok, it seems npm install --no-prune handles it.

I'll leave this open to update the documentation. (however this seems like potentially dangerous default behavior, since it'd break packages by removing anything that's not in an explicit dependency list.)

@ljharb

This comment has been minimized.

Copy link
Contributor Author

@ljharb ljharb commented Jun 1, 2017

Actually scratch that; it's still auto-pruning even with --no-prune - the auto-save behavior was complicating my debugging.

ljharb added a commit to airbnb/enzyme that referenced this issue Jun 1, 2017
@zkat zkat added the npm5 label Jun 1, 2017
@ivan-kleshnin

This comment has been minimized.

Copy link

@ivan-kleshnin ivan-kleshnin commented Jun 1, 2017

NPM 5 started to erase symlinks meant to polyfill absolute imports. This is very undesirable for me.

@ljharb

This comment has been minimized.

Copy link
Contributor Author

@ljharb ljharb commented Jun 17, 2017

(ping, just because i have no idea when npmbot might annihilate my issue)

@npm-robot

This comment has been minimized.

Copy link

@npm-robot npm-robot commented Jul 10, 2017

We're closing this support issue as it has gone three days without activity. The npm CLI team itself does not provide support via this issue tracker, but we are happy when users help each other here. In our experience once a support issue goes dormant it's unlikely to get further activity. If you're still having problems, you may be better served by joining package.community and asking your question there.

For more information about our new issue aging policies and why we've instituted them please see our blog post.

@npm-robot npm-robot closed this Jul 10, 2017
@danielkalen

This comment has been minimized.

Copy link

@danielkalen danielkalen commented Jul 10, 2017

This is still an issue... :/

@ljharb

This comment has been minimized.

Copy link
Contributor Author

@ljharb ljharb commented Jul 10, 2017

sigh. it'd be great if this could be reopened; auto-pruning is pretty destructive behavior.

ping @KenanY

@benderTheCrime

This comment has been minimized.

Copy link

@benderTheCrime benderTheCrime commented Jul 11, 2017

any word on reopening this? I just got bitten by this. Will settle for a --no-prune flag, although, I think perhaps Jordan is right about auto-pruning being potentially damaging behavior.

@ScottFreeCode

This comment has been minimized.

Copy link
Contributor

@ScottFreeCode ScottFreeCode commented Jul 17, 2017

I can see the motivation for wanting things pruned more frequently to prevent accidentally depending on things that aren't listed in the dependencies. But this is particularly nasty in combination with:

  • not printing what was pruned, so if I didn't think to npm ls beforehand I don't know what I lost and can't easily assess why it was there and whether it's still needed
  • doing it on npm install <package>, so it happens when I'm trying to upgrade a dependency in an already installed project
@ivan-kleshnin

This comment has been minimized.

Copy link

@ivan-kleshnin ivan-kleshnin commented Jul 18, 2017

@ScottFreeCode exactly

doing it on npm install , so it happens when I'm trying to upgrade a dependency in an already installed project

👍

If npm install really needs to prune node_modules I'm fine – automatic postinstall is still an option to recreate those symlinks and other stuff I wish to add to node_modules.

But npm install xxx both skipping postinstall but invoking prune mechanics is really outraging. Because now we need to manually run npm run postinstall after every manual npm install xxx.

npm behaves like it's the only owner of node_modules. But it's no longer the case after so many years of manually patching NPM and Node defects. There are tools, development approaches, deployment strategies and whatever else based on the node_modules folder and possibility to modify it. And you guys just starting to go against the community and ecosystem with this unasked "feature" of auto-pruning. Sorry for being so direct.

@ljharb

This comment has been minimized.

Copy link
Contributor Author

@ljharb ljharb commented Aug 3, 2017

@KenanY any chance this could be reopened? It's still a serious issue.

@KenanY

This comment has been minimized.

Copy link
Collaborator

@KenanY KenanY commented Aug 3, 2017

Not sure if this should be labelled bug or feature-request, it's up to the core team 😄

@KenanY KenanY reopened this Aug 3, 2017
@ljharb

This comment has been minimized.

Copy link
Contributor Author

@ljharb ljharb commented Aug 3, 2017

Thanks‼

I'd say it's both :-) "bug" because it doesn't account for symlinks and other pre-existing items in node_modules (not sure if it handles bundledDependencies); "feature request" to disable the behavior.

@ivan-kleshnin

This comment has been minimized.

Copy link

@ivan-kleshnin ivan-kleshnin commented Aug 3, 2017

I am personally fine if you keep new auto-prune but fire postinstall after EVERY install, not just basic npm install. It makes this hook more powerful (you can opt-out by argv conditioning) but it's not 100% backwards compatible. Well... the current behavior is not either.
So I'd like to see more people reporting their cases actually. Let's just not pretend some random option is obviously correct.

@smrq

This comment has been minimized.

Copy link

@smrq smrq commented Aug 3, 2017

This behavior turns lerna bootstrap --hoist into an exercise in misery.

@ScottFreeCode

This comment has been minimized.

Copy link
Contributor

@ScottFreeCode ScottFreeCode commented Aug 3, 2017

Personally, I'd like to run npm prune && npm test before publishing and on demand. I can't think of a reason to associate pruning with installing other packages. The latter doesn't necessarily ensure that I don't publish a package that was depending on unlisted dependencies (whereas the former does) and the latter does get in the way of experimenting with dependencies temporarily.

@danielkalen

This comment has been minimized.

Copy link

@danielkalen danielkalen commented Aug 10, 2017

Having this 'feature' completely defeats the purpose of the --no-save flag as any following npm install will have those packages removed.

Here's an example where auto-pruning is destructive: I have a package that has run-scripts that rely on a couple of external packages. These packages require on-install compiling from source and therefore take a long time to install. The scripts that depend on those external packages only rarely need to be run only in dev env (e.g. once in a few months). So in order to avoid 10-min installs on every CI run I removed these packages from devDependencies and had them installed by a task runner only when needed. But now with npm v5.x this workaround is completely counterintuitive as pretty much any npm command I run after removes those packages installed by the task runner.

Also, symlink'd packages seem to always break some how because of auto-pruning (I know it's because of auto-pruning because I forked npm and removed the auto-pruning feature and that has fixed the issue, for me at least).

@danielkalen

This comment has been minimized.

Copy link

@danielkalen danielkalen commented Aug 10, 2017

@ScottFreeCode you can just add a pretest hook/script to your package.json to prune prior to every test run

@ScottFreeCode

This comment has been minimized.

Copy link
Contributor

@ScottFreeCode ScottFreeCode commented Aug 10, 2017

Right, or a prepublishOnly hook/script to prune and then test if I want to be able to test without pruning under some circumstances.

The larger point being: pruning in a manner that's actually helpful (to ensure that I don't publish something that's depending on things I left out of package.json) doesn't even need to be builtin, let alone fire on unrelated actions such as installing new dependencies.

@smrq

This comment has been minimized.

Copy link

@smrq smrq commented Nov 16, 2017

Honestly... I don't love that? The thing with this feature is, it feels to me like npm is overstepping its bounds. It's true that npm is the de facto way of maintaining your node_modules folder, but it's still node_modules, not npm_modules. I feel like npm should respect that it might not be the only thing touching your node_modules folder, at least by default.

That said, it seems you could flip your suggestion and npm could mark dependencies as having been npm-installed, which gives you the same information (logically negated) automagically.

@darkobits

This comment has been minimized.

Copy link

@darkobits darkobits commented Nov 16, 2017

That said, it seems you could flip your suggestion and npm could mark dependencies as having been npm-installed, which gives you the same information (logically negated) automagically.

However, in some cases a third-party tool (ex: Lerna) is using the NPM client to install packages, so all packages in node_modules are still "NPM-installed". And, when a developer runs npm install some-package (assuming NPM4, pre auto-save) to install an extraneous package, they are still using NPM to do it.

So the issue is not only what to do with code in node_modules that didn't get there via NPM, it's what to do with modules that NPM considers extraneous according to package.json.

How would you all feel about some way of specifying a dependency as manually installed in node_modules, as a way to resolve this?

I think @substack's PR (#18921) is a nice solution to this, but what were you thinking?

@junkert

This comment has been minimized.

Copy link

@junkert junkert commented Nov 17, 2017

I like #18921 but the default should be to not prune. If you want to prune then you should add it in your configuration or add the flag. No package manager in existence uninstalls (at least a good one) without warning by default (or at least asking "Are you sure you want me to do this?")

image

@NextZeus

This comment has been minimized.

Copy link

@NextZeus NextZeus commented Nov 18, 2017

i degrade npm version to 4.x . this problem is so boring.

@mattgaspar

This comment has been minimized.

Copy link

@mattgaspar mattgaspar commented Nov 18, 2017

I can't believe this hasn't been addressed...is this the demise of npm? Why would anyone ever want a tool to make changes that were not requested by the command issued (with no way to prevent it)? Seems like something Microsoft would do!
The default should be to never auto-prune on any command.

And why when I install a package that has zero dependencies do I get a message that says 100+ packages were added, removed and updated? My package-lock file shows only 2 changes (the package I installed and not sure about the other, which was a dependency of an unrelated package). I've never installed any packages outside of npm in this project repo.

Example:

npm i interact.js -S

  • interact.js@1.2.8
    added 116 packages, removed 384 packages and updated 31 packages in 17.798s
@metasansana

This comment has been minimized.

Copy link

@metasansana metasansana commented Nov 21, 2017

@iarna is there documentation anywhere about this change?

@goldhand

This comment has been minimized.

Copy link

@goldhand goldhand commented Dec 4, 2017

@iarna that would be better than nothing. I still strongly dislike this feature and would really like to know what the thinking behind this was.
I don't think I work at the only company that uses it's own, internal, package management. Our internal package manager behaves similarly to npm but uses it's own manifest file (instead of package.json) and is very slow, so I end up using npm to install dependencies in development, before adding them to the internal tool's manifest. With this new change, all the packages that are managed using the internal tool's manifest are uninstalled.

Sad!

@smrq

This comment has been minimized.

Copy link

@smrq smrq commented Dec 8, 2017

Is there any reason not to go forward with @substack's PR? #18921

@cmg-dev

This comment has been minimized.

Copy link

@cmg-dev cmg-dev commented Dec 11, 2017

PR #18921 would also fix the issue for me and my teams.

@iarna

How would you all feel about some way of specifying a dependency as manually installed in node_modules, as a way to resolve this?

I could live with this. But isn't it just a way to whitelist things in you node_modules folder?

I would then suggest calling it differently, as it will be abused to do things with not-manually jobs as well ;)

My suggestion: exclude_from_autoprune.

@heikomat

This comment has been minimized.

Copy link

@heikomat heikomat commented Dec 12, 2017

Here a small test-script.

  • running it with npm 4 gives me the correct express-version both times
  • running it with npm 5 removes express during the weinre-install, so i don't get any express version at all the second time
echo '{"name": "a", "version": "1.0.0","dependencies": {"weinre": "^2.0.0-pre-I0Z7U9OV"}}' > package.json

npm install --no-save express
cat node_modules/express/package.json | grep version
#  npm4: "version": "4.16.2"
#  npm5: "version": "4.16.2"

npm install --no-save weinre
cat node_modules/express/package.json | grep version
#  npm4: "version": "4.16.2"
#  npm5: cat: node_modules/express/package.json: No such file or directory
@heikomat

This comment has been minimized.

Copy link

@heikomat heikomat commented Dec 12, 2017

@junkert i tried the changes from the commit you mentioned, but it didn't seem to help

@cmg-dev

This comment has been minimized.

Copy link

@cmg-dev cmg-dev commented Dec 12, 2017

@junkert Why is 0bea588 a fix?

Looks like it is just concerned with handling file locks and a gentle deletion.

We tested it and it does not change the behaviour at all.

@darkobits

This comment has been minimized.

Copy link

@darkobits darkobits commented Dec 14, 2017

@iarna,

This thread is now almost 7 months old. I think the participants have clearly outlined the problem, identified the root cause, and have suggested several workable solutions, including a PR.

I think it would be helpful if a core contributor could chime in and, at the very least, provide some indication of NPM's disposition here; what are your thoughts, what do you intend to do or not do, and what is the approximate timeline. This will help the people affected determine what their best course of action will be.

Thank you. 😸

@cmg-dev

This comment has been minimized.

Copy link

@cmg-dev cmg-dev commented Dec 15, 2017

@iarna (I don't know who else to address/summon)

Could you please provide feedback, why this isn't accepted?
My team and clients, depending on this, really can't understand this!

Some input would be tremendously helpful.

@junkert

This comment has been minimized.

Copy link

@junkert junkert commented Dec 16, 2017

@cmg-dev Thank you for verifying. You are correct it does not fix the issue.

What happened to my original post?

@AndreiShostik

This comment has been minimized.

Copy link

@AndreiShostik AndreiShostik commented Feb 12, 2018

I don't understand why now under the hood npm prune --production - #19806 - breaks implicitly all symlinks which is not desirable for all of devs as I understand. did someone ask about change request of this feature?

@darkobits

This comment has been minimized.

Copy link

@darkobits darkobits commented Feb 22, 2018

Looks like this might have been resolved in v5.7.0 (related blog post).

See BIG FIXES TO PRUNING section. Specifically, these two commits: 8279515 3b305ee.

@metasansana

This comment has been minimized.

Copy link

@metasansana metasansana commented Feb 24, 2018

Do not install v5.7.0. Skip straight to 5.7.1. It has a pretty nasty bug.

@codinronan

This comment has been minimized.

Copy link

@codinronan codinronan commented Feb 25, 2018

For anyone coming here from Cordova, I can confirm that 5.7.1 fixes the issues where npm decides to blow away the node_modules of any cordova-plugins. I have been tearing my hair out over this for weeks.

@goldhand

This comment has been minimized.

Copy link

@goldhand goldhand commented Mar 8, 2018

AHHHH YAAAASSS. Its fixed. Finally, npm leaves my poor node_modules/ alone...

Using >=5.7.0 with package lock disabled will no longer auto prune.

Disable package lock globally:

$ npm config set package-lock false
@eliot-akira

This comment has been minimized.

Copy link

@eliot-akira eliot-akira commented May 10, 2018

This is still an issue with npm@6.0.0, one symptom being that npm install clobbers ("auto-prunes") any local modules symlinked via npm link. I've been struggling with the same issue since last year, forcing me to migrate to yarn.

Just discovered the work-around by @goldhand to disable package-lock globally, which prevents symlinked modules from being deleted or replaced by modules from the registry. Finally I can get back to a saner workflow with npm using symlinked local modules.

Still not a real solution though, and I hope this issue gets the attention it needs.

@legodude17

This comment has been minimized.

Copy link
Contributor

@legodude17 legodude17 commented May 10, 2018

See npm/rfcs#3 for the link problem.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.