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.

Sign up for GitHub

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

Jump to bottom

[BUG] uninstall does not remove --no-save installed package from node_modules #2309

Closed
raphinesse opened this issue Dec 9, 2020 · 2 comments · Fixed by npm/arborist#194
Closed

[BUG] uninstall does not remove --no-save installed package from node_modules #2309

raphinesse opened this issue Dec 9, 2020 · 2 comments · Fixed by npm/arborist#194
Assignees
@isaacs
Labels
Bug thing that needs fixing Release 7.x work is associated with a specific npm 7 release semver:patch semver patch level for changes

Comments

@raphinesse
Copy link

raphinesse commented Dec 9, 2020 •
edited
Loading

Current Behavior:

If I install a package using the --no-save option, it is not removed from node_modules/ upon its uninstallation.

Expected Behavior:

If I install a package using the --no-save option, it should get removed from node_modules/ upon its uninstallation.

NB: this is also how npm 6 behaves.

Steps To Reproduce:

  1. Run the following
cd $(mktemp -d)
mkdir node_modules
npm i q --no-save
npm un q --no-save
  1. Observe that node_modules/q still exists.

Omitting the --no-save option for the uninstallation command yields the same result. Substituting mkdir node_modules with npm init --yes makes no difference either.

Environment:

  • OS: Ubuntu 18.04.5 & 19.10
  • Node: 12.19.0 & 15.4.0
  • npm: 7.0.15 & 7.1.1
@raphinesse raphinesse added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Dec 9, 2020
@raphinesse raphinesse mentioned this issue Dec 9, 2020
Merged
6 tasks
@isaacs
Copy link
Contributor

isaacs commented Dec 13, 2020

Confirmed. This appears to be a bug in how we're handling the fake {} package we use when there's no package.json in the root project, combined with using the actualTree as the effective starting idealTree when there's no package-lock.json present. If there's either a package.json or a package-lock.json that declares the deps present, it does properly remove them. If neither of those are there, it doesn't properly remove the dependency.

@isaacs isaacs mentioned this issue Dec 13, 2020
Closed
@isaacs isaacs self-assigned this Dec 14, 2020
@isaacs isaacs added semver:patch semver patch level for changes and removed Needs Triage needs review for next steps labels Dec 14, 2020
isaacs added a commit to npm/arborist that referenced this issue Dec 15, 2020
@isaacs
Reset flags when initializing ideal from actual
1e0b489 
Fix: #193
Fix: npm/cli#2309
@isaacs isaacs mentioned this issue Dec 15, 2020
Merged
@nlf nlf mentioned this issue Dec 15, 2020
Merged
raphinesse added a commit to raphinesse/cordova-fetch that referenced this issue Dec 16, 2020
@raphinesse
ci: use npm7 version that fixed npm/cli#2309
ba2e7f3
raphinesse added a commit to raphinesse/cordova-fetch that referenced this issue Dec 16, 2020
@raphinesse
ci: use npm7 version that fixed npm/cli#2309
dace3fc
@MylesBorins MylesBorins mentioned this issue Dec 16, 2020
Closed
raphinesse added a commit to apache/cordova-fetch that referenced this issue Jan 26, 2021
@raphinesse
fix: cordova-fetch with npm@7 (#91)
d001b88 
* fix: determine installed package's name on npm 7

* test: make expectations work for npm 5 to 7

This addresses the following changes in behavior.

Saved GitHub URL format in package.json:
- npm 6: git+https://github.com/apache/cordova-android.git#4.1.x
- npm 7: github:apache/cordova-android#4.1.x

Empty devDependencies format in package.json:
- npm 6: `{}`
- npm 7: `undefined`

* ci: add node@15 w/ npm@7

* ci: use npm7 version that fixed npm/cli#2309
@miafan23
Copy link

Is there any plan to fix this bug in npm6 also? Would love to see it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@isaacs isaacs

Labels
Bug thing that needs fixing Release 7.x work is associated with a specific npm 7 release semver:patch semver patch level for changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Reset flags when initializing ideal from actual npm/arborist
4 participants
@isaacs @raphinesse @miafan23 and others