feat(migrate): upgrade existing Vite+ projects across versions#1891
Conversation
✅ Deploy Preview for viteplus-preview canceled.
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
feb8068 to
5090afc
Compare
vite-plus
@voidzero-dev/vite-plus-core
@voidzero-dev/vite-plus-prompts
@voidzero-dev/vite-plus-cli-darwin-arm64
@voidzero-dev/vite-plus-cli-darwin-x64
@voidzero-dev/vite-plus-cli-linux-arm64-gnu
@voidzero-dev/vite-plus-cli-linux-arm64-musl
@voidzero-dev/vite-plus-cli-linux-x64-gnu
@voidzero-dev/vite-plus-cli-linux-x64-musl
@voidzero-dev/vite-plus-cli-win32-arm64-msvc
@voidzero-dev/vite-plus-cli-win32-x64-msvc
@voidzero-dev/vite-plus-darwin-arm64
@voidzero-dev/vite-plus-darwin-x64
@voidzero-dev/vite-plus-linux-arm64-gnu
@voidzero-dev/vite-plus-linux-arm64-musl
@voidzero-dev/vite-plus-linux-x64-gnu
@voidzero-dev/vite-plus-linux-x64-musl
@voidzero-dev/vite-plus-win32-arm64-msvc
@voidzero-dev/vite-plus-win32-x64-msvc
commit: |
5090afc to
732edd6
Compare
E2E test projects
× typescript(TS2783): 'staged' is specified more than once, so this usage will be overwritten.
╭─[vite.config.ts:118:3]
117 │ export default defineConfig({
118 │ ╭─▶ staged: {
119 │ │ '*': 'vp check --fix',
120 │ ╰─▶ },
121 │ ...config, // shared lint/fmt/build from @janosh/vite-config (dotfiles)
╰────
✖ bash -c "tsc --noEmit":
error TS2688: Cannot find type definition file for 'vite-plus/client'.
The file is in the program because:
Entry point of type library 'vite-plus/client' specified in compilerOptions
tsconfig.json:12:33
12 "types": ["vitest/globals", "vite-plus/client"],
~~~~~~~~~~~~~~~~~~
File is entry point of type library specified here.
|
This comment was marked as outdated.
This comment was marked as outdated.
7a1d2de to
c68f763
Compare
1bba44c to
911881e
Compare
|
@codex review |
…+ outro The existing-Vite+ path's no-migrate `else` branch printed the hint and outro without stopping the "Checking config compatibility" spinner that checkWorkspaceRolldownCompatibility left running, so its timer line kept re-rendering underneath them (a ghost spinner after "Happy coding!"). Clear it first, matching the sibling format and summary branches. Claude-Session: https://claude.ai/code/session_01DQhS6o1fyQd1yjiee6W8jR
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7da2ece3dd
ℹ️ 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".
An existing Vite+ project's version-update prompted to set up pre-commit hooks whenever a legacy husky/lint-staged config was present, unlike agent/editor setup which already defer to --full. For an unsupported lint-staged format the prompt could never do anything (it always ended in "skipping git hooks setup"). getExistingVitePlusSetupOptions now defaults hooks to false like agent/editor; legacy husky/lint-staged migration runs only with `vp migrate --full` or an explicit `--hooks`. Drops the now-unused legacyGitHooksMigrationCandidate param and variable. Claude-Session: https://claude.ai/code/session_01DQhS6o1fyQd1yjiee6W8jR
…igrate.sh The version report resolved the package manager via `vp env current | vp node`, but the isolated install runs with VP_NODE_MANAGER=no, so `vp node` has no managed Node and the parse returned empty: no `-r`, so `vp why` queried the workspace root without recursion and reported nothing. Detect pnpm from the project's own markers (a `packageManager` pin, a workspace file, or the post-install lockfile) instead. For pnpm, report all three packages in one recursive query (`vp why -r vite-plus vite vitest`); npm/yarn/bun still loop one package at a time since their `why` accepts only one. Claude-Session: https://claude.ai/code/session_01DQhS6o1fyQd1yjiee6W8jR
Two fixes to the vitest-injection guard in rewritePackageJson (PR review): - The injection fallback matched any dep whose name contains "vitest", including VITEST_DIRECT_USAGE_EXCLUDED packages like @vitest/eslint-plugin. When such a package drove the only signal, a catalog monorepo omitted the vitest catalog entry yet this still injected `vitest: catalog:`, a dangling spec that breaks pnpm install. Use isVitestAdjacent (which applies the exclusion filter). - A @nuxt/test-utils package keeps its `from 'vitest'` imports (the rewriter preserves them for Nuxt), but the guard never added a direct vitest, so under strict pnpm/Yarn layouts the preserved imports could not resolve. Add hasNuxtTestUtilsDependency to needDirectVitest and the injection condition. Adds two reproducing migrator tests. Claude-Session: https://claude.ai/code/session_01DQhS6o1fyQd1yjiee6W8jR
|
@codex review |
Picks up voidzero-dev/pkg-pr-registry-bridge#51, which fixes preview tarballs missing the tar end-of-archive marker (ERR_PNPM_TARBALL_EXTRACT: "Invalid checksum for TAR header") when the packed size is an exact multiple of 10240. Claude-Session: https://claude.ai/code/session_01DQhS6o1fyQd1yjiee6W8jR
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5cf9447af4
ℹ️ 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".
…r gate, preview registry) Addresses four Codex review comments on PR #1891, all verified valid: - install.sh: `resolve_bridge_commit_version ... || true` so `set -e` does not abort at the assignment before the "Could not resolve" error prints (the installer otherwise fails silently on an unregistered/transient bridge miss). - test-pkg-pr-new-migrate.sh: only force-stage `.npmrc`/`.yarnrc.yml` that carry the bridge marker, not a pre-existing ignored file that may hold private registry auth tokens the migration never touched. - migration/bin.ts: detect Prettier unconditionally on existing-Vite+ upgrades so the format gate skips Oxfmt on a Prettier project even on a non-`--full` upgrade; the Prettier migration itself stays gated on `--full`. - preview-registry.ts: thread the detected package manager into reconcilePreviewBridgeRegistry so a pnpm/npm/bun project with a stray `.yarnrc.yml` still gets `.npmrc` (its install reads `.npmrc`). Adds a test. Claude-Session: https://claude.ai/code/session_01DQhS6o1fyQd1yjiee6W8jR
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab9a0b36e6
ℹ️ 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".
A standalone (non-monorepo) migration on pnpm 9.5.0-10.6.1 rewrote the toolchain edges to `catalog:` (catalogs are supported from 9.5.0) but never wrote the catalog entries to pnpm-workspace.yaml: that write was gated on `usePnpmWorkspaceYaml` (settings support, >= 10.6.2), so the install could not resolve `catalog:` and failed. Gate the pnpm-workspace.yaml write on `usePnpmWorkspaceYaml || supportCatalog` and pass `usePnpmWorkspaceYaml` as the `writeWorkspaceSettings` flag, so a 9.5-10.6.1 project gets a catalog-only file (settings stay in package.json), mirroring the monorepo orchestrator. 452cbf6 fixed the existing-Vite+ bootstrap path; this covers the standalone new-project path. Removes the now-redundant `shouldRewritePnpmWorkspaceYaml` state (it only ever mirrored `usePnpmWorkspaceYaml`). Adds a reproducing test. Claude-Session: https://claude.ai/code/session_01DQhS6o1fyQd1yjiee6W8jR
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! 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". |
Rewrite the page in descriptive voice (only Before You Migrate stays imperative) so readers can tell user steps from automatic behavior. Merge Dependency Versions/Changes into one Dependency Rules section with an at-a-glance table, split Source Rewrite Rules into scannable subsections, turn the script command mapping into a table, group the pnpm rules by topic, and move the Rolldown warning and reinstall/fmt behavior into a new After the Migration section. No behavioral rules were added, removed, or weakened.
Correctness fixes from the xhigh workflow review of this branch vs main: - vite-plus-bootstrap: write the pnpm-workspace.yaml catalog (creating the file when missing) on standalone pnpm 9.5-10.6.1 upgrades so reconciled catalog: specs resolve, and judge catalog: specs/overrides through the workspace catalog in the <10.6.2 pending check so a migrated project converges to the already-using-Vite+ fast exit - compat runner: kill the config-resolution worker after 30s (new runCommandSilently timeoutMs) instead of hanging vp migrate when a project-local vite-plus predates the lazyPlugins-skip handshake - npm reinstall: move stale node_modules/vite aside and restore it when the final npm install fails, instead of deleting it up front - format: only fall back to whole-project formatting on a genuine "not a git repository"; dubious-ownership/git-missing now skip formatting; cap format arg batches at 24KB on Windows (32,767-char CreateProcess limit) - catalog: survive scalar-valued peerDependencyRules entries in pnpm-workspace.yaml instead of crashing mid-write - import rewriter: rewrite declare module 'vite' augmentations in every file again (they must track the rewritten config import identity) while keeping the #2004 config-only scoping for value imports and reference directives; count triple-slash-only preserved vitest references in preserved_vitest_files - package detection: verify an ancestor .pnp.cjs actually owns the project (findPackageLocator) before running setup(), so foreign PnP hooks are never installed process-wide - preview registry: stash a custom Yarn Berry npmRegistryServer in a YAML comment and restore it on real-release cleanup instead of deleting it - prompts: stop interpolating huge path batches into the vp fmt failure hint - oxlint plugin: bypass the @nuxt/test-utils cache when stat fails instead of colliding on the mtimeMs 0 sentinel
…import
Regenerate the migration-rewrite-declare-module snapshot for the review fix
that rewrites declare module 'vite' augmentations in every file again: an
augmentation binds to the literal specifier, so it must target the same
module as the rewritten vite.config.ts import ('vite-plus') or the augmented
UserConfig options fail type-checking. Value imports and reference-type
directives keep the #2004 config-files-only scoping, and vitest augmentations
are still never rewritten.
Also document the augmentation exception in migrate-rules.md.
Partially reverts e29973c. Through the core alias a preserved declare module 'vite' augmentation reaches the same @voidzero-dev/vite-plus-core module whose UserConfig types defineConfig from vite-plus, so it keeps working after migration. vite-plus exports no UserConfig symbol, so the rewritten declare module 'vite-plus' augmentation merged with nothing and silently stopped applying. Users extending vite-plus's own surface write declare module 'vite-plus' directly. Restores the #2004 config-files-only scoping for all vite specifiers, flips the regression test to assert preservation, regenerates the migration-rewrite-declare-module snapshot, and documents the rationale in migrate-rules.md.
|
AI review and fixed done, human test pass, waiting for CI pass then merge. 🤖 |
RFC:
rfcs/migrate-existing-projects.mdProblem
vp migrateon an existing v0.1.x Vite+ project did not upgrade cleanly: it delegated to the stale local CLI, leftpnpm-workspace.yamloverrides and catalogs pinningvite/vitestto old versions, and skewed coverage providers. The v0.2.x release notes currently tell users not to runvp migrateyet.What it does
vite-plusis older than the globalvp, run migrate from the global CLI so the stale local CLI does not run.vite-plusand thevite->@voidzero-dev/vite-plus-corealias off 0.1.x, across dependencies, overrides/resolutions, and catalogs, for every workspace package rather than only the root manifest.@vitest/*(coverage-v8/-istanbul, ui, web-worker) to the bundled version, with the browser providers kept opt-in.package.json#pnpmsettings intopnpm-workspace.yaml, fix the empty"pnpm": {}misrouting that left stale overrides, inject the directviteedge where pnpm needs it, and approve required build scripts. The "pending" check scans every workspace package's catalog ref, so a package pinned to a stale named catalog (catalog:legacy) is not missed.vite/vitestimports invite-plugin-*and unplugin packages instead of rewriting them tovite-plus.Updated . to Vite+ <version>(orMigratedfor a fresh project) and lists the toolchain version changes as afrom -> totable forvite-plus, the rawviteversion, andvitestwhen present.Checking config compatibility (N/total): <package>), so the spinner keeps moving and shows what it is processing instead of stalling on a stale message.Version-only by default,
--fullfor setupOn an existing Vite+ project,
vp migratedoes the toolchain version upgrade above and silently skips the first-time setup bucket (git hooks, editor config, agent files, ESLint/Prettier migration, framework shims, tsconfigbaseUrl, and.nvmrc/Volta to.node-version), so a version bump does not re-touch things you already set up. Pass--fullto run the whole setup, or a per-action flag (--hooks/--agent/--editor) to opt into a single action. A bare upgrade that left setup actions on the table hints atvp migrate --full. Fresh (non Vite+) projects always run the full migration.On a large monorepo the spinner walks the per-package config-compatibility check, so the run is never silent:
Node.js version
The native binding runs on any Node
>=20.0.0, and the published@voidzero-dev/vite-plus-*platform packages now declare that ABI floor (thevite-plusand core packages keep their widerengines.nodesupport range). So a Node pin whose floor is below the support range but at or above20.0.0(engines.node: 24.x,>=22,.node-version24.3.0, ...) still installs the native binding and is left untouched by migration. Converting.nvmrcand Voltavolta.nodeto.node-version(and repointing anyactions/setup-nodenode-version-file: .nvmrcreference in workflows and composite actions,.github/actions/**/action.{yml,yaml}, so CI does not break) is part of the--fullsetup, not the default upgrade.Manual pkg.pr.new testing
Install an isolated pkg.pr.new global CLI and run the PR's
vp migrateagainst a local project. Dependencies resolve through the registry bridge as ordinary0.0.0-commit.<sha>versions, persisted into the project's.npmrc(and.yarnrc.ymlfor Yarn Berry) so its own CI installs the same build:The first argument accepts a PR number or commit SHA. The helper keeps
~/.vite-plusuntouched, forces migration through the preview CLI even when the project has a same-version local CLI, clears only the workspace root lockfile andnode_modulesbefore migrating, and refuses dirty Git worktrees unlessALLOW_DIRTY=1. Confirm the result withvp why -r vite-plus vite vitest(each must resolve to a single version).