fix: use real package name for global local packages installation#1685
fix: use real package name for global local packages installation#1685liangmiQwQ wants to merge 7 commits into
Conversation
✅ Deploy Preview for viteplus-preview canceled.
|
![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: f017f86dd6
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1daa2779f
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1daa2779f
ℹ️ 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".
| if is_local_package_spec(package_name) { | ||
| // We can't resolve local packages for uninstall, follow npm's behavior | ||
| return Err(Error::ConfigError( | ||
| format!( |
There was a problem hiding this comment.
This early return blocks vp remove -g ./local-path before any metadata/bin lookup, which breaks uninstall for packages installed by older versions that stored the local spec path as the managed package key (the behavior this commit is replacing). After upgrading, those existing installs can no longer be removed via vp by either path (now rejected here) or name (metadata/bin ownership still recorded under the old path), leaving orphaned shims and package state that can block subsequent installs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Maybe it will influence vp install -g --force's uninstall logic, but won't influence normal vp uninstall -g <pkg_name>, as we do not use BinConfig there. I've tested locally and it can still remove a global package
$ vp install -g ./conflict-pkg
VITE+ - The Unified Toolchain for the Web
info: Installing 1 global package with Node.js 24.16.0
warn: Package './conflict-pkg' provides 'node' binary, but it conflicts with a core shim. Skipping.
✓ Installed ./conflict-pkg 1.0.0
Bins: conflict-cli, node
~/code/voidzero-dev/vite-plus on fix/check-real-package-json-for-global-install [!]
$ pnpm bootstrap-cli
...
~/code/voidzero-dev/vite-plus on fix/check-real-package-json-for-global-install [!] took 1m37s
$ vp uninstall -g conflict-pkg
Uninstalled conflict-pkg
tip: Available short aliases: i = install, rm = remove, un = uninstall, up = update, ls = list, ln = link
~/code/voidzero-dev/vite-plus on fix/check-real-package-json-for-global-install [!]
$ vp list -g conflict-pkg
No global packages matching 'conflict-pkg'.
Run 'vp list -g' to see all installed global packages.1 participant
Related to #664.
The current Vite+ will just treat the path inputed as the package name, and because Vite+ uses package name to create directories and manage installation, it will make Vite+ broken as it can be a bad path.
This PR fixes it by resolving the true package name by reading the
package.jsonunder the entered path.