fix(migrate): dedupe vite/vitest under pnpm by adding a direct vite dep#1933
Merged
Conversation
✅ Deploy Preview for viteplus-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
fengmk2
changed the title
fengmk2
force-pushed
the
fix/1932-pnpm-vite-dedupe
branch
from
121b36a to
4de2889
Compare
Member
Author
|
@codex review |
fengmk2
added
test: e2e
![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: 4de2889644
ℹ️ 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".
fengmk2
changed the title
Under pnpm, `vp create vite:monorepo` resolves vite-plus / vite / vitest to 2 instances each (the workspace root and packages/utils auto-install an upstream vite to satisfy vitest's peer, since they have no direct vite for the override to bind). This step asserts a single instance via `vp why` and is expected to fail for the monorepo + pnpm matrix entry, reproducing the bug before the fix. Refs #1932
Print the complete `vp why <pkg>` output (not just the summary lines) for any package that resolves to multiple instances, so the reproduction job shows the full dependency tree that explains the split. Refs #1932
Under pnpm, any importer that depends on vite-plus (which bundles vitest) needs a direct `vite` dep so the `vite` override binds vitest's required `vite` peer to @voidzero-dev/vite-plus-core. Without a direct edge, pnpm's autoInstallPeers installs a separate upstream vite to satisfy the peer, splitting vite-plus / vite / vitest into duplicate instances (the extra vite also lacks vite's vite-task-client integration, breaking the vp test cache). Mirror the override as a direct vite devDep in the workspace root (rewriteRootWorkspacePackageJson) and every package (rewritePackageJson), pnpm-only. npm/yarn/bun redirect the transitive/peer vite via root overrides/resolutions, so they are untouched. Closes #1932
The direct-`vite` dedupe fix makes the bundled vitest resolve `vite` to @voidzero-dev/vite-plus-core (which carries the vite-task-client integration) regardless of which upstream version the age gate would pick, so the PNPM_CONFIG_MINIMUM_RELEASE_AGE=0 workaround is no longer needed. The "Verify single dependency instances" step is now a passing regression guard rather than a known-failing reproduction. Refs #1932
Apply oxfmt to the new migrator blocks, hoist the test pkg literal out of a factory to satisfy oxlint, and regenerate the global migration snapshots that now include the direct `vite` devDep for pnpm vite-plus consumers. Refs #1932
- Extract `ensureDirectViteForPnpm` and reuse it for the root, per-package, and standalone migration paths (was duplicated, and the standalone path was missing entirely, so a pnpm project adopting vite-plus without a prior vite still reproduced the duplicate-instance bug). - Skip injection when `vite` is declared only as a peerDependency, so a vite plugin's peer contract (e.g. `vite ^6`) is preserved instead of getting a conflicting concrete devDep (reverts the two skip-vite-peer snapshots). - Guard against a custom VP_OVERRIDE_PACKAGES without a `vite` key, which crashed the root branch with a TypeError. - Treat any declared vite spec (including an empty string) as already-direct. - Drop an em-dash from a new comment. Refs #1932
Drop the inert getCatalogDependencySpec options (dependencyName / packageManager / catalogDependencyResolver) - they only affect an existing value or a peerDependencies field, neither of which applies to the fresh devDependencies entry this helper writes - and the now-unused catalogDependencyResolver parameter. Add the `vite` key in place via `??=` to match the sibling npm branch instead of rebuilding the object. No behavior change.
The rationale lives in the ensureDirectViteForPnpm JSDoc; drop the repeated explanation (and #1932 tag) from the three call sites, keeping only the one non-obvious note about the standalone path needing a second pass.
Use `vp why -r` so the guard inspects every workspace package, not just the root importer, catching a duplicate confined to a sub-package. Addresses the Codex review comment.
oxfmt sorts package.json dependencies, but the #1932 helper appended `vite` last. `vp create` re-formats afterward so it was fine, but `vp migrate` has no format pass, so a migrated project failed a follow-up `vp check` (caught by the vp-config E2E). Insert `vite` in sorted position instead; regenerate the affected migration snapshots.
fengmk2
force-pushed
the
fix/1932-pnpm-vite-dedupe
branch
from
4de2889 to
f73fdbe
Compare
cpojer
approved these changes
Jun 25, 2026
wan9chi
approved these changes
Jun 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1932.
Under pnpm, a created/migrated monorepo resolved vite-plus / vite / vitest to 2 instances each: packages depending on vite-plus without a direct
vite(the workspace root andpackages/utils) left vitest'svitepeer for pnpm's autoInstallPeers to satisfy with a second, upstream vite. That upstream vite lacks vite's vite-task-client integration, which broke thevp testcache.Fix (pnpm-only, in the migrator): wherever vite-plus is depended upon, add a direct
vitedevDep aliased to the override target so vitest's peer binds to @voidzero-dev/vite-plus-core. npm/yarn/bun already redirect the transitive/peer vite via root overrides/resolutions, so they are untouched.Also adds a "Verify single dependency instances" CI guard, drops the temporary
PNPM_CONFIG_MINIMUM_RELEASE_AGE=0band-aid from #1774, and covers the cases with unit tests.