refactor: replace @voidzero-dev/vite-plus-test with upstream vitest@4.1.5#1588
refactor: replace @voidzero-dev/vite-plus-test with upstream vitest@4.1.5#1588Brooooooklyn wants to merge 15 commits into
Conversation
✅ Deploy Preview for viteplus-preview canceled.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![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: 365a61de42
ℹ️ 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".
| const pkgJsonPath = resolve('vitest/package.json'); | ||
| const pkgRoot = dirname(pkgJsonPath); | ||
| const pkgJson = JSON.parse(readFileSync(pkgJsonPath, 'utf-8')) as VitestPackageJson; | ||
| const binRel = typeof pkgJson.bin === 'string' ? pkgJson.bin : pkgJson.bin?.vitest; | ||
| if (!binRel) { | ||
| throw new Error(`Could not find 'vitest' bin entry in ${pkgJsonPath}`); | ||
| } | ||
| const binPath = join(pkgRoot, binRel); |
There was a problem hiding this comment.
Switching vp test to run upstream vitest directly removes the wrapper behavior that previously made vite-plus/test compatible with mock hoisting. With this resolver, projects that still import vi from vite-plus/test can hit non-hoisted vi.mock(...) calls (Vitest hoists only for the literal 'vitest' source), so mocked modules may load before the mock is applied and tests can silently execute against real implementations.
Useful? React with 👍 / 👎.
…mat/typo PR #1588 CI failures: - Force-override mode (VP_FORCE_MIGRATE=1, set by test-vp-create.yml and ecosystem-ci) now re-pins any pre-existing vite-plus range to the local tgz path in monorepo workspace packages. Without this, pnpm reads the published vite-plus@0.1.21 metadata to resolve transitive deps including @voidzero-dev/vite-plus-test@0.1.21, which shadowed upstream vitest@4.1.5 at runtime and broke vp create monorepo tests. - typos CI: rename yarn-PnP to yarn Plug'n'Play (Pn→On false positive). - vp check: format packages/cli/build.ts shim-generation block. - Rename __dirname in install-failure-guard.spec.ts to satisfy eslint(no-underscore-dangle).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fafa67971
ℹ️ 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".
| const BROWSER_PROVIDER_PACKAGES: ReadonlyArray<{ pkg: string; short: string }> = [ | ||
| { pkg: '@vitest/browser-playwright', short: 'playwright' }, | ||
| { pkg: '@vitest/browser-preview', short: 'preview' }, | ||
| { pkg: '@vitest/browser-webdriverio', short: 'webdriverio' }, |
There was a problem hiding this comment.
The new export sync only mirrors vitest plus provider packages, but not @vitest/browser, so vite-plus/test/browser/context is no longer produced even though migration still rewrites to it (see packages/cli/snap-tests-global/migration-from-vitest-config/snap.txt, which imports userEvent from that path). In Node, a missing export (or types-only context mapping) causes ERR_PACKAGE_PATH_NOT_EXPORTED, so migrated browser-mode configs/tests can fail at runtime immediately after this change.
Useful? React with 👍 / 👎.
| name: 'vitest', | ||
| pattern: /vitest-dev: npm:vitest@\^([\d.]+(?:-[\w.]+)?)/, | ||
| replacement: `vitest-dev: npm:vitest@^${versions.vitest}`, | ||
| // The `@voidzero-dev/vite-plus-test` wrapper (which used to be aliased | ||
| // here via `vitest-dev: npm:vitest@^…`) has been removed. Vitest is now | ||
| // a plain catalog entry pinned to an exact version (`vitest: x.y.z`), | ||
| // so match that shape directly. The leading newline anchor disambiguates | ||
| // from neighbouring keys like `vitepress-*` and `@vitest/browser`. | ||
| pattern: /\n {2}vitest: ([\d.]+(?:-[\w.]+)?)\n/, | ||
| replacement: `\n vitest: ${versions.vitest}\n`, |
There was a problem hiding this comment.
After this refactor, pnpm-workspace.yaml now pins @vitest/browser, @vitest/browser-playwright, @vitest/browser-preview, and @vitest/browser-webdriverio, but updatePnpmWorkspace() only updates the vitest key. The daily upgrade workflow can therefore advance Vitest while leaving browser packages stale, creating version skew in published deps and avoidable install/peer-resolution breakage in CI and release automation.
Useful? React with 👍 / 👎.
| installDurationMs: initialInstallSummary.durationMs + finalInstallSummary.durationMs, | ||
| installDurationMs: | ||
| handleInstallResult(initialInstallSummary, workspaceInfo.rootDir, report) + | ||
| finalInstallDurationMs, |
There was a problem hiding this comment.
Initial install failure overwrites final install exit code
Medium Severity
handleInstallResult(initialInstallSummary, ...) is called inside the return expression at line 829, AFTER handleInstallResult(finalInstallSummary, ...) at line 822. Since handleInstallResult sets process.exitCode on failure, if the initial install failed but the final install succeeded, the initial install's error code overwrites the process exit state, making a successful migration appear to have failed. The initial install summary needs to be reconciled before or independently of the final install, not after it.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ec69aba. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec69abaadb
ℹ️ 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".
| "./test/browser-playwright/context": { | ||
| "types": "./dist/test/browser-playwright/context.d.ts" | ||
| }, |
There was a problem hiding this comment.
vp migrate now rewrites runtime imports to vite-plus/test/browser-playwright/context (see packages/cli/snap-tests-global/migration-from-vitest-files/snap.txt, line 39), but this export entry only declares a types path and no default/import/require target. Any migrated test that does import { server } from 'vite-plus/test/browser-playwright/context' will fail module resolution at runtime, so browser-mode projects can break immediately after migration.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 381b6e2c20
ℹ️ 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".
| ) { | ||
| const ver = VITE_PLUS_OVERRIDE_PACKAGES.vitest; | ||
| pkg.devDependencies.vitest = getCatalogDependencySpec(undefined, ver, supportCatalog); | ||
| pkg.devDependencies.vitest = getCatalogDependencySpec(undefined, '4.1.5', supportCatalog); |
There was a problem hiding this comment.
When isVitestAdjacent is true, this branch injects devDependencies.vitest via getCatalogDependencySpec(undefined, '4.1.5', supportCatalog), which returns "catalog:" for catalog-capable package managers. Since this commit removed vitest from VITE_PLUS_OVERRIDE_PACKAGES, rewriteCatalog(...) no longer creates a catalog.vitest entry, so migrated projects can end up with vitest: "catalog:" but no matching catalog key (the updated migration-vitest-peer-dep snapshot shows exactly that shape). In pnpm this fails installation with ERR_PNPM_CATALOG_ENTRY_NOT_FOUND_FOR_SPEC, so migration leaves dependencies broken in vitest-adjacent projects that don't already define a vitest catalog entry.
Useful? React with 👍 / 👎.
….1.5
Delete the bundled `@voidzero-dev/vite-plus-test` wrapper and consume
upstream `vitest@4.1.5` (plus `@vitest/browser*`) directly. The vite
redirection that the wrapper used to provide is now handled cleanly
by package-manager overrides (`vite` -> `@voidzero-dev/vite-plus-core`),
so the bundle was dead weight that lagged upstream vitest releases.
Public API contract preserved:
- `vite-plus/test*` IS the public test API. Existing vite-plus user
code (e.g. `import { vi } from 'vite-plus/test'`) is NEVER rewritten —
the imports stay exactly as authored.
- New vite-plus users do NOT install `vitest` or `@vitest/*` directly.
They install `vite-plus`; vitest and the browser providers come in
transitively as direct deps of the CLI package.
- `vp migrate` on an upstream-vitest project still rewrites bare
`vitest`, `vitest/*`, `@vitest/browser*`, declare-module specifiers
and `/// <reference types>` directives to the `vite-plus/test*`
surface (one-time forward migration).
Implementation:
- packages/cli/build.ts `syncTestPackageExports` auto-generates
`./test/*` shims from `vitest`'s own exports map, plus
`./test/<provider>` and `./test/browser/providers/<short>` shims
projected from each `@vitest/browser-*` package's exports.
- packages/cli/package.json adds `@vitest/browser`,
`@vitest/browser-playwright`, `@vitest/browser-preview`,
`@vitest/browser-webdriverio` as direct catalog deps pinned to
4.1.5 alongside `vitest`.
- crates/vite_global_cli/src/commands/version.rs version ToolSpec
points at the `vitest` package directly.
- packages/cli/src/resolve-test.ts resolves `vitest/package.json`
and reads `bin.vitest` so `vp test` invokes upstream vitest.
- packages/cli/src/utils/constants.ts drops `vitest` from
`VITE_PLUS_OVERRIDE_PACKAGES`; only `vite` remains a managed key.
- packages/cli/src/migration/migrator.ts adds an `isVitestAdjacent`
flag that flips `needVitePlus = true` for projects with packages
like `vitest-browser-svelte` even when no vite/oxlint/tsdown dep
is being migrated; adds a `pruneLegacyWrapperAliases` /
`pruneYamlMapLegacyWrapperAliases` sweep that rewrites stale
`vitest: npm:@voidzero-dev/vite-plus-test@*` aliases to `^4.1.5`
in package.json overrides/resolutions/pnpm.overrides, in
pnpm-workspace.yaml catalog/catalogs/overrides, and in bun
workspaces.catalog / workspaces.catalogs.
- packages/cli/src/migration/bin.ts adds a `handleInstallResult`
helper so failed reinstalls warn the user, append to
`report.warnings`, and flip `process.exitCode` instead of being
silently reported as success.
Verification:
- cargo test -p vite_migration --lib: 167 tests pass
- pnpm exec vitest run (packages/cli): 355 tests pass
- pnpm bootstrap-cli + snap-test-global + snap-test-local: all
fixtures regenerated; expected diffs only (forward import
rewrites, `@vitest/browser*` removed from user devDeps,
`playwright`/`webdriverio` preserved as peers, stale
`vite-plus-test` catalog aliases normalized to `^4.1.5`).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…mat/typo PR #1588 CI failures: - Force-override mode (VP_FORCE_MIGRATE=1, set by test-vp-create.yml and ecosystem-ci) now re-pins any pre-existing vite-plus range to the local tgz path in monorepo workspace packages. Without this, pnpm reads the published vite-plus@0.1.21 metadata to resolve transitive deps including @voidzero-dev/vite-plus-test@0.1.21, which shadowed upstream vitest@4.1.5 at runtime and broke vp create monorepo tests. - typos CI: rename yarn-PnP to yarn Plug'n'Play (Pn→On false positive). - vp check: format packages/cli/build.ts shim-generation block. - Rename __dirname in install-failure-guard.spec.ts to satisfy eslint(no-underscore-dangle).
…r migration The migration step intentionally rewrites overrides/catalogs/deps so the existing lockfile is guaranteed to be stale. pnpm and yarn default to frozen-lockfile mode under CI, so they refuse the reinstall with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH and the new handleInstallResult enforcement propagates that failure as a non-zero exit. Add --no-frozen-lockfile alongside the existing --force for npm/bun, applied to both the post-migration reinstall and the ESLint/Prettier early-return reinstall.
…r types through vite-plus, prune monorepo aliases Three independent fixes for E2E failures introduced by the vite-plus-test → upstream-vitest refactor: 1. Stop rewriting `declare module 'vitest'` (and subpaths/@vitest variants). The `vite-plus/test*` shims re-export upstream `vitest*`, so the type identity is upstream. Augmenting `'vite-plus/test'` only augments the shim and never merges into the upstream types the user actually sees through `import { expect } from 'vite-plus/test'`. Import rewrites stay; declare-module rewrites are dropped. 2. Inline-copy upstream `@vitest/browser-*` d.ts content into the test shims and rewrite `vitest/node`/`vitest/browser`/`@vitest/browser*` bare specifiers to relative paths inside the `dist/test/` tree. Without this, pnpm splits `@vitest/browser-playwright` and `vitest` across different peer-dep edges, creating two `BrowserProvider` type identities — and `provider: playwright()` fails the user's vite.config typecheck. 3. Add the same `pruneLegacyWrapperAliases` calls to `rewriteRootWorkspacePackageJson` that `rewriteStandaloneProject` already had. Monorepo migrations were leaving stale `npm:@voidzero-dev/vite-plus-test@*` aliases in root resolutions / overrides / pnpm.overrides, causing pnpm to install the deleted wrapper. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…talls vitest@4.1.5 declares a `vite^6/^7/^8` peer dep. When the user's project overrides `vite` to `@voidzero-dev/vite-plus-core` (version 0.0.0 in dev and 0.1.21 on npm — neither matches), bun aborts with: error: vite@^6.0.0 || ^7.0.0 || ^8.0.0 failed to resolve pnpm/yarn/npm tolerate this redirect; bun does not, and offers no `peerDependencyRules`-style escape hatch — only `[install] peer = false` in `bunfig.toml`. vite-plus already provides the vite surface the user needs, so disabling bun's auto-install of *missing* peers is safe here: the redirected vite is the only one that ever fails, and other vitest peers (jsdom, happy-dom, @vitest/*, etc.) are upstream-optional and either pulled in transitively or user-installed. `ensureBunfigPeerSuppression` writes/merges `bunfig.toml` whenever the migrator touches a bun project — both the monorepo path (via `rewriteBunCatalog`) and the standalone path (in `rewriteStandaloneProject`'s post-package.json branch). Honors any pre-existing `peer =` setting so a user who deliberately set `peer = true` keeps it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… behavior
Earlier drafts of this branch claimed the migrator stopped rewriting `vitest`
runtime imports and that the browser-provider subpaths were no longer
re-exported. That landed in docs/guide/migrate.md, rfcs/migration-command.md,
and packages/cli/BUNDLING.md, but the implementation kept the forward rewrites
(`vitest` → `vite-plus/test`, `@vitest/browser*` → `vite-plus/test/browser*`)
and added the inlined-d.ts shim infrastructure that *does* re-export every
provider subpath under both `./test/<pkg>` and `./test/browser/providers/<short>`.
The only thing actually preserved is `declare module 'vitest'` / `declare
module '@vitest/browser*'` — those have to target the upstream module identity
to merge into the types `vite-plus/test*` re-exports.
This commit restores the migrate guide's "import { vi } from 'vite-plus/test'"
example, restores the full rewrite-rule list in the migration-command RFC,
fixes the bundling doc's "provider subpaths are not re-exported" claim, and
adds a short subsection explaining why the provider d.ts shims are inlined
instead of bare re-exports (the pnpm-edge type-identity split that would
otherwise break user `provider: playwright()` typechecks).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Brooooooklyn
force-pushed
the
refactor/replace-vite-plus-test-with-vitest
branch
from
5c48da8 to
39efcbf
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 39efcbf. Configure here.
| plan.packageManager === PackageManager.npm || plan.packageManager === PackageManager.bun | ||
| ? ['--force'] | ||
| : undefined; | ||
| : ['--no-frozen-lockfile']; |
There was a problem hiding this comment.
Yarn receives unsupported --no-frozen-lockfile pnpm flag
High Severity
The else branch passes ['--no-frozen-lockfile'] for all non-npm/non-bun package managers, which includes both pnpm and yarn. --no-frozen-lockfile is a pnpm-specific flag — Yarn v4 (berry) does not recognize it (the equivalent is --mode update-lockfile or disabling enableImmutableInstalls). The old code passed undefined for pnpm/yarn, avoiding this entirely. Now both the full migration path and the early-return ESLint/Prettier path send this unsupported flag, which will cause vp install to fail or error on yarn-managed projects.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 39efcbf. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39efcbf239
ℹ️ 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".
| replace: vitest/ | ||
| by: "vite-plus/test/" |
There was a problem hiding this comment.
The catch-all rule rewrites any vitest/* import to vite-plus/test/*, but this commit removed many legacy ./test/* deep exports (for example ./test/plugins/runner is no longer in packages/cli/package.json). That means projects using historical subpaths like vitest/plugins/runner will now be migrated to vite-plus/test/plugins/runner and fail at runtime with ERR_PACKAGE_PATH_NOT_EXPORTED. Either limit rewrites to subpaths that vite-plus actually exports or keep compatibility exports for the rewritten deep paths.
Useful? React with 👍 / 👎.
* `.github/workflows/test-vp-create.yml`: bun strictly checks override target versions against transitive peer ranges; vitest 4.1.5 declares peer `vite ^6 || ^7 || ^8` and bun aborts when the override points at the 0.0.0 tgz. Repack core with version `7.99.0` so the satisfied range is met, restore the source `package.json` before packing the CLI, and rename the tgz back to the 0.0.0 filename the env var references. * `snap-tests-global/migration-rewrite-declare-module/snap.txt`: align with the preserved-declare-module rewrite behavior (declare module 'vitest' / 'vitest/config' stay as-is; only `declare module 'vite'` still rewrites to 'vite-plus').
The previous attempt repacked core in place and restored the source package.json before packing the CLI; this left the CLI tgz declaring @voidzero-dev/vite-plus-core@0.0.0 while the override target's internal version was 7.99.0. install-global-cli (which resolves the CLI's transitive @voidzero-dev/vite-plus-core@0.0.0 dep) then asked npm for that exact version on the registry — which doesn't exist — and every non-bun vp-create job started failing. Build the bun-friendly variant as a separate tgz instead: - core-0.0.0.tgz stays untouched (install-global-cli + CLI's transitive dep both resolve against it as before) - core-bumped.tgz holds the same contents with the internal version rewritten to 7.99.0 so bun's strict peer-dep check on the override target passes - VP_OVERRIDE_PACKAGES (in test-vp-create.yml matrix env and ecosystem-ci/patch-project.ts) points at the bumped variant Background: vitest 4.1.5 declares peer `vite ^6 || ^7 || ^8` and bun strictly validates the override target's version against that range where pnpm/npm/yarn don't. See oven-sh/bun#8406.
Previous attempt extracted core-0.0.0.tgz, rewrote the inner version, repacked with raw \`tar czf\` and pointed VP_OVERRIDE_PACKAGES at that variant. Two problems with that: * On Windows GNU tar in Git Bash mis-parses the D:\\ path as a remote host and aborts (broken pipe), so the Build vite-plus packages job failed before tgz upload. * The raw \`tar czf\` repack appears to drop something pnpm pack would have included (the bumped tgz was ~200KB smaller than the original for a one-byte version change) and bun continued to report \`vite@^6.0.0 || ^7.0.0 || ^8.0.0 failed to resolve\` against the override target — likely because the tgz wasn't a clean npm package. Switch to bumping packages/core/package.json#version to a synthetic 7.99.0 *before* both pnpm pack calls so: * core tgz is produced by pnpm pack with the bumped internal version * cli tgz declares @voidzero-dev/vite-plus-core@7.99.0 (workspace:* resolved at pack time) — install-global-cli reads the CLI's transitive dep and looks for the matching tgz, which now exists. The source package.json is restored via git after both packs so the working tree is clean for downstream steps. VP_OVERRIDE_PACKAGES (in test-vp-create.yml env and ecosystem-ci/patch-project.ts) is updated to point at the new \`voidzero-dev-vite-plus-core-7.99.0.tgz\` filename.
vp check failed in CLI E2E test on the previous commit because the provider-exports markdown table separator row was a few dashes short of matching the widest cell. Run vp check --fix to re-align.
Bun walks transitive peer-deps before resolving overrides. vitest@4.1.5 declares peer \`vite ^6 || ^7 || ^8\` and aborts with "vite@... failed to resolve" if \`vite\` isn't a direct dep somewhere in the tree, even when the user's overrides would redirect it to vite-plus-core. pnpm, npm, and yarn don't enforce this; bun is uniquely strict. See oven-sh/bun#8406. For both standalone and monorepo-root bun migrations, mirror the \`vite\` override as a top-level devDependency so bun's resolver sees a direct \`vite\` dep at install time. The override configured alongside still redirects the resolved package at vite-plus-core, so the user's actual import behavior is unchanged — they still get vite-plus-core for \`import 'vite'\`. The only snap.txt that needed updating is new-vite-monorepo-bun, which scaffolds a fresh bun monorepo without an existing \`vite\` dep at the root. migration-monorepo-bun and create-from-bun-monorepo-subdir already start with \`vite\` in devDeps and are unaffected.
Mirroring the override target as a devDep wasn't enough — bun also validates that the resolved package's package.json#name matches the requested specifier. The file: target was @voidzero-dev/vite-plus-core, not vite, so bun still aborts with "vite@^6.0.0 || ^7.0.0 || ^8.0.0 failed to resolve" even after the devDep injection landed in 61289fb. Pack a second tgz with name=vite, version=7.99.0 (same code as core) and point VP_OVERRIDE_PACKAGES.vite at it. The user's package.json now gets: overrides.vite = file:.../vite-7.99.0.tgz (name=vite) overrides.@voidzero… = file:.../core-7.99.0.tgz (name=core) devDependencies.vite = file:.../vite-7.99.0.tgz (name=vite) which satisfies both of bun's checks; pnpm/npm/yarn don't care. Three pack steps now run sequentially: 1. set name=vite + version=7.99.0, pack core → vite-7.99.0.tgz 2. restore name=@voidzero-dev/vite-plus-core, pack core → voidzero-dev-vite-plus-core-7.99.0.tgz 3. pack cli (workspace:* → core@7.99.0 so install-global-cli matches) 4. git checkout the source package.json
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2fceee5296
ℹ️ 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".
| envs: Record<string, string>; | ||
| }> { | ||
| const binPath = join(dirname(resolve('@voidzero-dev/vite-plus-test')), 'dist', 'cli.js'); | ||
| const pkgJsonPath = resolve('vitest/package.json'); |
There was a problem hiding this comment.
vp test against bundled vitest to avoid skew
Resolving vitest/package.json from process.cwd() first means vp test can execute a project-local Vitest version while vite-plus/test shims still re-export the Vitest copy bundled under vite-plus. In pnpm/yarn PnP setups this can produce two different Vitest instances (for example, local vitest@4.2.x plus bundled 4.1.5), leading to runner/API mismatches and hard-to-debug failures in config/plugins/runtime behavior. This regression is introduced by switching binary resolution to cwd-first instead of the CLI-owned dependency.
Useful? React with 👍 / 👎.
…ng during pack The previous masquerade approach renamed packages/core/package.json between two pnpm pack calls. Even with the name restored before packing the CLI, pnpm's workspace state was polluted: catalog `vite: workspace:@voidzero-dev/vite-plus-core@*` resolved to literal `vite@7.99.0` in the packed CLI manifest, and pnpm then failed at install time with `ERR_PNPM_NO_MATCHING_VERSION` looking up vite@7.99.0 in the npm registry. Pack core (version bumped to 7.99.0, name unchanged) and the CLI normally, then run a new `tool repack-vite-tgz` subcommand that reads the core tgz, rewrites the inner `package.json#name` to "vite", and writes a sibling `vite-7.99.0.tgz`. The repacked tgz still satisfies bun's strict name+version peer check for vitest 4.1.5's `peer vite ^6 || ^7 || ^8`, but no workspace pollution occurs because we never modify the in-tree package name.
Pointing every package manager at the renamed vite-7.99.0.tgz worked around bun's strict peer check but broke pnpm: when both a workspace sub-package and the workspace override resolve to the same tgz that advertises itself as vite@7.99.0, pnpm tries to validate the version against the registry and aborts with ERR_PNPM_NO_MATCHING_VERSION. Revert to the main-branch shape for pnpm/npm/yarn (same core tgz for both `vite` and `@voidzero-dev/vite-plus-core` overrides) and keep the renamed tgz only for bun, where vitest's `peer vite ^6 || ^7 || ^8` demands a tgz whose package.json#name is "vite" and whose version is in range. Stop bumping core/CLI to 7.99.0 entirely; pack at 0.0.0 and post-process a sibling vite-7.99.0.tgz via `tool repack-vite-tgz`, which now also strips the workspace's vite→vite-plus-core self-ref so the alias tgz isn't circular. The test-vp-create matrix picks the override target per package manager via a new VITE_OVERRIDE_TGZ matrix env. ecosystem-ci/patch-project.ts mirrors that logic for the bun-vite-template project only; every other ecosystem project routes vite through the core tgz.
Apply formatting fixes flagged by `vp check`: oxfmt added braces around single-line if-bodies in build.ts and reflowed the new repack-script helper and ecosystem-ci patch ternary. No behavior change.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b888329d76
ℹ️ 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".
| !installableNames.includes('vitest') && | ||
| adjacentSignals.some((name) => name !== 'vitest' && name.includes('vitest')); |
There was a problem hiding this comment.
isVitestAdjacent currently excludes packages that already list vitest (!installableNames.includes('vitest')), so rewritePackageJson() can leave needVitePlus false for workspace packages that depend on vitest but not vite/@vitest/browser*. In the monorepo flow, those package manifests rely on rewritePackageJson() to add vite-plus, while imports are still rewritten to vite-plus/test* later; that combination produces rewritten source with no declared vite-plus dependency, which can fail resolution in strict package-manager setups (notably per-package installs or PnP/isolation).
Useful? React with 👍 / 👎.
1 participant
Summary
Deletes the bundled
@voidzero-dev/vite-plus-testwrapper and consumes upstreamvitest@4.1.5(plus@vitest/browser*) directly. The vite redirection role that drove the wrapper is now handled cleanly by package-manager overrides (vite→@voidzero-dev/vite-plus-core), so the bundle was dead weight that lagged upstream releases.Public API contract preserved:
vite-plus/test*IS the public test API — existing user code (import { vi } from 'vite-plus/test', etc.) is NEVER rewritten.vitestor@vitest/*separately; they come in transitively as direct deps ofvite-plus.vp migrateon an upstream-vitest project still forward-migratesvitest,vitest/*,@vitest/browser*, declare-module specifiers, and/// <reference types>directives to thevite-plus/test*surface (one-time transition).Notable changes:
packages/cli/build.ts:syncTestPackageExportsauto-generates./test/*shims from upstreamvitest's exports map, plus./test/<provider>and./test/browser/providers/<short>shims projected from each@vitest/browser-*package's exports.packages/cli/package.json: adds@vitest/browser,@vitest/browser-playwright,@vitest/browser-preview,@vitest/browser-webdriverioas direct catalog deps pinned to4.1.5.crates/vite_global_cli/src/commands/version.rs: vitest ToolSpec points at thevitestpackage directly.packages/cli/src/resolve-test.ts: resolvesvitest/package.jsonand readsbin.vitestsovp testinvokes upstream vitest.packages/cli/src/utils/constants.ts: dropsvitestfromVITE_PLUS_OVERRIDE_PACKAGES; onlyviteremains a managed key.packages/cli/src/migration/migrator.ts:isVitestAdjacentflag that flipsneedVitePlus = truefor projects with packages likevitest-browser-svelteeven when there's no vite/oxlint/tsdown to migrate.pruneLegacyWrapperAliases/pruneYamlMapLegacyWrapperAliasessweeps that rewrite stalevitest: npm:@voidzero-dev/vite-plus-test@*aliases to^4.1.5(so existingcatalog:refs keep resolving) and drop other stale wrapper-targeted keys.packages/cli/src/migration/bin.ts: adds ahandleInstallResulthelper so failed reinstalls warn the user, append toreport.warnings, and flipprocess.exitCodeinstead of being silently reported as success.User-visible behavior changes
vp test -hand live test runs now show vitest's native banner (vitest/<semver>,RUN v<semver> <cwd>) instead of the wrapper-rebranded output (vp test/<semver>,RUN <cwd>). This is the tradeoff for delegating directly to upstream vitest without a wrapper layer.Test plan
cargo test -p vite_migration --lib: 167 tests passpnpm exec vitest run(packages/cli): 374 tests passpnpm bootstrap-clisucceedspnpm -F vite-plus snap-test-global+snap-test-local: all fixtures regenerated; diffs only reflect expected behavior (forward import rewrites,@vitest/browser*removed from user devDeps,playwright/webdriveriopreserved as peers, stalevite-plus-testcatalog aliases normalized to^4.1.5).vp teston a fixture usingimport { vi } from 'vite-plus/test';vi.mock(...). See "Follow-up" below.vp migrateon a fresh upstream-vitest project.pnpm installclean: zero@voidzero-dev/vite-plus-testreferences in the lockfile, browser-provider packages installed transitively viavite-plus.Follow-up
@vitest/mockerhoistsvi.mock(...)calls only when the import source is the literal string'vitest'(@vitest/mocker@4.1.5/dist/chunk-hoistMocks.jshardcodeshoistedModule = "vitest"). User code that importsvifrom'vite-plus/test'won't get its mocks hoisted, which silently breaks mocking. The plan is to contribute upstream ahoistedModule?: string | string[]option tohoistMocks()so vite-plus can pass['vitest', 'vite-plus/test']. Tracked as a separate effort — this PR ships the wrapper removal independently; the mocker fix can land later as a@vitest/mockerpoint release.🤖 Generated with Claude Code
Note
Medium Risk
Medium risk because it removes the
@voidzero-dev/vite-plus-testpackage and rewires CLI shims, migration logic, and CI packaging around upstreamvitest/@vitest/browser*, which can affect installs and type resolution (especially for bun/CJS export conditions).Overview
Removes the bundled
@voidzero-dev/vite-plus-testwrapper and makesvite-plus/test*shims re-export directly from upstreamvitest(plus@vitest/browser-*providers), updating CLI exports, dependencies, and documentation accordingly.CI/release workflows are adjusted to stop packing/publishing the removed test package, and ecosystem/bun paths gain a special
vitetgz alias + bun install settings to satisfy bun’s strictvitestpeer-dep checks.Migration/import rewriting is updated to (a) scan
.cjs/.ctsfiles, (b) stop rewritingdeclare module 'vitest*'/@vitest/*augmentations, (c) stop injectingvitestoverrides/catalog aliases, and (d) properly detect/report install failures via a newhandleInstallResulthelper; snapshots and tests are refreshed and new regression tests added for exports-map condition ordering.Reviewed by Cursor Bugbot for commit b888329. Bugbot is set up for automated code reviews on this repo. Configure here.