Repository navigation
fix(shim): preserve npm global links during uninstall dry runs - #3112
Conversation
Only clean up links after the package disappears from the selected npm prefix. Collect uninstall binaries from that prefix without falling back to another Node installation. Cover dry runs configured through CLI flags, environment variables, and user npmrc, while verifying that real uninstalls still remove links.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d536bb4bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Recognize -C and -C= so link creation and cleanup use the same prefix as npm when npmrc or NPM_CONFIG_PREFIX points elsewhere. Cover argument ordering, mixed-prefix precedence, configuration overrides, and dry-run preservation.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d94400848
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Keep the package path collected before uninstall so a surviving local copy cannot be mistaken for the removed global installation. Verify that real uninstall cleans up global links while preserving local packages, and that dry runs still preserve the installed command.
Restore lookup in the selected Node installation when the configured npm prefix has no readable package manifest. Retain the actual installation path so dry runs and no-op uninstalls preserve links to surviving packages. Reuse the existing package lookup helper without adding npm configuration parsing. Cover userconfig overrides with a private runtime copy so tests do not modify the shared runtime seed.
Replace the private runtime copy script with seed-runtime = false and a pinned Node version. Keep the package and link assertions and verify that the runtime directory is not a shared symlink before global npm writes. This case now downloads its own runtime rather than copying the shared seed.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bdd2c8c13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Restore a private copy of the seeded runtime so global npm writes stay isolated without downloading Node for each case. Point the Node distribution mirror at a closed local endpoint to catch accidental download dependencies. Keep the existing uninstall assertions and regenerate the snapshot.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
fengmk2
added
test: e2e
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93385e58bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
2 participants
Summary
npm uninstall -g <package> --dry-runleaves the package installed, but Vite+ previously removed its executable links and BinConfig after npm exited successfully. This could make the installed command unavailable through Vite+.Only clean up links after the package has disappeared from the selected npm prefix. This also preserves installed state when dry-run is enabled through
.npmrcornpm_config_dry_run, without duplicating npm's configuration parsing.Collect uninstall binaries only from the selected prefix to avoid removing links based on a package installed under another Node prefix.