Skip to content
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

[BUG] npm ci does not remove/recreate existing node_modules inside workspaces #7226

Closed
2 tasks done
thislooksfun opened this issue Feb 16, 2024 · 3 comments · Fixed by #7490
Closed
2 tasks done

[BUG] npm ci does not remove/recreate existing node_modules inside workspaces #7226

thislooksfun opened this issue Feb 16, 2024 · 3 comments · Fixed by #7490
Assignees
Labels
Bug thing that needs fixing Good First Issue good issue or PR for newcomers Priority 2 secondary priority issue

Comments

@thislooksfun
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

Running npm ci behaves unintuitively in workspaces. When run in a project without workspaces, the behavior is straightforwards and clear:

  1. Validate that the package-lock.json is in sync
  2. Remove ./node_modules if it exists
  3. Install from package-lock.json

This makes perfect sense, and aligns with the documentation. However, when you run npm ci in a project with workspaces, it runs the same three steps. This sounds like it's fine, except that there is one key problem with step 2: it only deletes the root node_modules folder. Any node_modules in workspaces folders are left behind. This leads to a confusing inconsistency in the behavior of the npm ci command: on first install (if there are no node_modules installed at all), it will install the full tree, including workspaces. However, when running it a second time, it only cleanly reinstalls the root packages.

Expected Behavior

I would expect that npm ci would remove all the node_modules folders referenced in package-lock.json before trying to install again.

Steps To Reproduce

Here is a set of commands that illustrate the issue. If you run this script you will see that the hoisted abbrev installation gets properly reinstalled (removing foo.txt), but the inner one does not.

npm init -y
npm init -y -w workspace-a
npm init -y -w workspace-b

npm i --save-exact abbrev@1.1.0 -w workspace-a
npm i --save-exact abbrev@1.1.1 -w workspace-b

touch node_modules/abbrev/foo.txt
touch workspace-b/node_modules/abbrev/bar.txt

npm ci

# This will _not_ include foo.txt, as it has been deleted.
ls node_modules/abbrev
# This _will_ include bar.txt, as it has _not_ been deleted.
ls workspace-b/node_modules/abbrev

Environment

  • npm: 10.4.0 and 9.8.1
  • Node.js: v18.18.0
  • OS Name: macOS
  • System Model Name: Macbook Pro
  • npm config:
save-exact = true 
save-prefix = ""
@thislooksfun thislooksfun added Bug thing that needs fixing Needs Triage needs review for next steps Release 10.x labels Feb 16, 2024
@thislooksfun
Copy link
Author

This is a possible duplicate of #4846, but that one was a slightly different situation (specifying a workspace to reinstall, rather than running npm ci bare), and also was closed almost 2 years ago, so I figured it would be better to open this as a new issue.

@milaninfy milaninfy added Priority 2 secondary priority issue and removed Needs Triage needs review for next steps labels Mar 12, 2024
@wraithgar wraithgar added the Good First Issue good issue or PR for newcomers label Mar 13, 2024
@kalil0321
Copy link

Hi, I would like to contribute.

@DesignByOnyx
Copy link

The ci stands for "clean install" - the behavior should live up to the name. If someone needs similar behavior to work cross-platform, you can use rimraf --glob ./**/node_modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Good First Issue good issue or PR for newcomers Priority 2 secondary priority issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants