feat(mocker): hoist vi.mock() for vite-plus/test imports#10489
Conversation
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
![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: 2699b6170c
ℹ️ 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".
|
How do you forward the option? Technically this plugin is used internally so the options are not publicly exposed unless you inject your own version of this plugin? |
Brooooooklyn
force-pushed
the
feat/mocker-configurable-hoist-module
branch
from
2699b61 to
35bd12e
Compare
Brooooooklyn
changed the title
|
You're right — I've dropped the option and instead recognize const REDISTRIBUTED_HOISTED_MODULES = ['vite-plus/test']
// ...
if (hoistedModule === source || REDISTRIBUTED_HOISTED_MODULES.includes(source)) {If you'd rather not bake a specific specifier into core — happy to switch to a generic mechanism instead (e.g. a config option plumbed through |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35bd12e3e6
ℹ️ 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".
sheremet-va
left a comment
sheremet-va
left a comment
There was a problem hiding this comment.
I think I'm fine with this. @hi-ogawa what do you think?
`vite-plus` re-exports Vitest's mocking API as `vite-plus/test`. Recognize
that specifier as the hoisted module so `import { vi } from 'vite-plus/test'`
followed by `vi.mock(...)` is statically hoisted exactly like `vitest`,
instead of being left unhoisted (silent breakage / TDZ at runtime).
Recognition only — the injected import for files that use the API without
importing it is unchanged. This upstreams the pnpm patch vite-plus currently
ships to do the same thing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Brooooooklyn
force-pushed
the
feat/mocker-configurable-hoist-module
branch
from
35bd12e to
f82f39a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f82f39a7aa
ℹ️ 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".
…1588) ## Summary Deletes the bundled `@voidzero-dev/vite-plus-test` wrapper and consumes upstream `vitest@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. - New vite-plus users don't install `vitest` or `@vitest/*` separately; they come in transitively as direct deps of `vite-plus`. - `vp migrate` on an upstream-vitest project still forward-migrates `vitest`, `vitest/*`, `@vitest/browser*`, declare-module specifiers, and `/// <reference types>` directives to the `vite-plus/test*` surface (one-time transition). **Notable changes:** - `packages/cli/build.ts`: `syncTestPackageExports` auto-generates `./test/*` shims from upstream `vitest`'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-webdriverio` as direct catalog deps pinned to `4.1.5`. - `crates/vite_global_cli/src/commands/version.rs`: vitest 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 there's no vite/oxlint/tsdown to migrate. - Adds `pruneLegacyWrapperAliases` / `pruneYamlMapLegacyWrapperAliases` sweeps that rewrite stale `vitest: npm:@voidzero-dev/vite-plus-test@*` aliases to `^4.1.5` (so existing `catalog:` refs keep resolving) and drop other stale wrapper-targeted keys. - `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. ## User-visible behavior changes `vp test -h` and 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 - [x] `cargo test -p vite_migration --lib`: 167 tests pass - [x] `pnpm exec vitest run` (packages/cli): 374 tests pass - [x] `pnpm bootstrap-cli` succeeds - [x] `pnpm -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`/`webdriverio` preserved as peers, stale `vite-plus-test` catalog aliases normalized to `^4.1.5`). - [ ] Manual end-to-end: `vp test` on a fixture using `import { vi } from 'vite-plus/test'`; `vi.mock(...)`. See "Follow-up" below. - [ ] Manual end-to-end: `vp migrate` on a fresh upstream-vitest project. - [ ] `pnpm install` clean: zero `@voidzero-dev/vite-plus-test` references in the lockfile, browser-provider packages installed transitively via `vite-plus`. ## Follow-up - vitest-dev/vitest#10489 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches release/publish paths, daily dep bumps, and CI override behavior for all package managers (especially Bun peer resolution). Public migration guidance and version reporting change, but the `vite-plus/test*` import surface stays the same. > > **Overview** > Removes **`@voidzero-dev/vite-plus-test`** from release, pkg.pr.new, and CI pack flows, and stops bumping **`packages/test`** in **`upgrade-deps`** and prepare-release. Vitest is now tracked as upstream **`vitest`** with catalog **`@vitest/*`** entries kept on the same exact version. > > **Dependency automation** rewrites **`VITEST_VERSION`**, **`test-vp-create`** override pins, and README manual-migration **`vitest`** literals (exact semver instead of **`npm:@voidzero-dev/vite-plus-test@latest`**). **`pnpm-workspace.yaml`** matching switches from the old **`vitest-dev`** alias to a plain **`vitest:`** catalog line. > > **CI packaging** pins **`core`/`cli`** to **`0.0.0`** for stable tgz names, drops the test package tarball, and adds **`repack-vite-tgz`** so Bun smoke/e2e can satisfy Vitest’s **`peer vite`** via a masqueraded **`vite-7.99.0.tgz`**. **`vp create`** tests override **`vitest`/`@vitest/*`** by version instead of a local test tgz. > > **User-facing docs and CLI**: README explains pinning bundled Vitest for a single copy with **`vp test`**. **`vp --version`** resolves Vitest from the **`vitest`** package. Migration file walking adds **`.cjs`/`.cts`** for reverse **`require('vite-plus/test/...')`** rewrites. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 9b59731. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
3 participants
Description
Vite+ re-exports Vitest's mocking API as
vite-plus/test, so established users write:The static
vi.mock()hoister only recognizesvitestas the module that provides the mocking API, so for the specifier above thevi.mock()call is not hoisted — it runs after the import binding instead of before it, causing silent breakage / a TDZ error at runtime.This recognizes
vite-plus/testas a hoisted-module specifier alongsidevitest, sovi.mock()is hoisted for it exactly as it is forvitest. It's recognition-only (the injected import for files that use the API without importing it is unchanged) and requires no configuration.This upstreams the small pnpm patch vite-plus currently ships (which hardcodes the same
vite-plus/testcheck); landing it lets that patch be dropped.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yamlunless you introduce a new test example.Tests
pnpm test:ci.Documentation
Changesets
AI disclosure: prepared with assistance from Claude (Claude Code); reviewed and validated locally (build +
injector-mocksuite + eslint) before submitting.