-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FEATURE] Do not remove node_modules on npm ci #564
Comments
This is not what ci / cleaninstall is meant to do. The current behaviour is correct. What you want to use is |
We added an update to avoid deleting the node_modules folder but not its contents (as originally requested on that post). The |
Thanks for your replies! Sorry for my late reply. I've looked at @claudiahdz; My understanding is that running P.s. I thought |
As referenced here: npm/npm#20104 (comment) The current behavior is problematic if you're using It causes the following error:
which then results in aborting the Docker container. It'd be lovely to have a |
ci = clean install This is expected. Why don't you use the normal |
|
...because: https://docs.npmjs.com/cli/ci.html
The faster installs and clean slate approach make this ideal for CI environments such as the one I mentioned above. |
This is what I do now, but see above for the desire to use |
It seems reasonable to me to file an RFC asking for a config flag that makes |
Isn't this also what another issue described, to allow npm to create files to ignore the dir (for OSX spotlight and others)? I think there were also others who need this feature. |
|
I support this feature. As stated, |
same, a flag would be great |
Why not add a flag for |
What part of "clean install" is incompatible with keeping the parent I realize that CI doesn't stand for Continuous Integration in this case; but a Clean Install is often quite useful in a Continuous Integration environment, as the documentation makes clear.
The behavior of deleting the So we're asking for an option that will make this command useful for its intended purpose and environment. |
I have to ask this question are their any other differences between |
In my opinion this should only happen once when the image is built. After that |
Personally that makes more sense to me, additional flags for the normal |
I'm indifferent to which, and honestly too much of a n00b with js land to have a concrete argument that it must be |
So See https://blog.npmjs.org/post/171556855892/introducing-npm-ci-for-faster-more-reliable It covers a specific use case. For other use cases we should add new flags to |
Otherwise flags are spread over |
I'd be very happy if the feature was added to |
How is BTW this issue should get more attention and maintainers should voice their opinions and plans about that, using docker is basically always used in CI (continuous integration) which is one of the main use case of npm ci ! |
Please open an RFC for this change, rather than an issue in this repo. |
To avoid confusion, I’d rename this issue as “empty instead of remove node_modules dir in npm CI” |
My intention of this issue was never to delete the Maybe I'm wrong but I feel there are two issues here. Maybe someone could start another issue or RFC about deleting only contents of node_modules instead of deleting the folder completely? Or am I missing something? |
@Zenuka the entire reason npm CI is fast, and exists, is because it ignores the existing node_modules dir, so it’s pretty unlikely that will change. |
@nmm-shumway v6 to v7 is a breaking change, so a node major that ships v6 will always do so. LTS status has nothing to do with it. |
Should this bugfix be backported to v6 then? I still don't understand whether v6 ignoring the |
@nmm-shumway id hope so, but my guess is that it’s unlikely since v7 is completely rewritten. However users are expected to upgrade npm to the latest that works on their node version anyways, so if you’re on node 10+ you should be on npm 7+. |
Meh, this is fine. My org can upgrade the npm version we're using, so this doesn't really matter to me beyond curiosity. It seems a little bit weird to me that users are expected to upgrade across versions in an LTS if those versions have breaking changes, and if the breaking changes aren't a problem, it seems a little weird to me that they're being treated as a blocking issue preventing shipping inside the LTS distributions. But, again, I don't actually need a resolution on that, from what I can see in my own testing, v7 fixes the majority of the issues I'm having, so at least for my org the answer is definitely just to upgrade. I appreciate the responses, swapping to v7 should make our dev/testing builds a bit more reliable, so I'm pretty happy about that. |
What / Why
I would really like to see a flag like
npm ci --keep
to do an incremental update on our buildserver as this would speed up our deployments a lot. Like suggested before on github and on the community. The last update was on the 7th of October that this was being reviewed by the cli team. Could someone post an update on this? :-)The text was updated successfully, but these errors were encountered: