fix: forward-port #364 (jest-dom root resolution) + #323 (respect vitest projects environments) - #367
Merged
Merged
Conversation
…pace is set Forward-port of #323 (@carloitaben, fixes #205) to the 3.0.0-next line. A root vitest config that defines `test.projects` (or the pre-vitest-4 `test.workspace`) runs no tests itself; each project controls its own environment. Injecting the jsdom default at the root made vitest probe for (and prompt to install) jsdom on startup even when every project runs under node or in browser mode. Adds a config-resolution assertion to the start-ssr vitest suite.
Forward-port of #364 (@brenelz, fixes #231) to the 3.0.0-next line. getJestDomExport probed `@testing-library/jest-dom/vitest` with the plugin's own `require` (created from import.meta.url), so under pnpm's isolated node_modules it reported the package available whenever ANY package in the tree depended on jest-dom transitively (Storybook in the issue). Vitest resolves bare `setupFiles` from the project root, where it isn't installed, and failed with `Failed to load url .../@testing-library/jest-dom/vitest`. Divergence from #364: instead of a `createRequire` rooted at the project, the probe walks `<dir>/node_modules/@testing-library/jest-dom` up from the Vite root (the way Vite/Node resolve without NODE_PATH). `require.resolve` also consults NODE_PATH, and pnpm's bin shims (`pnpm vitest`, `pnpm test`) export NODE_PATH=node_modules/.pnpm/node_modules — the hoisted virtual store where every transitive dependency is reachable — so the root-derived require still found the transitive copy under the shim while vitest did not. The subpath (`/vitest`, then v5's `/extend-expect`) is then checked against that exact package copy. The bare specifier is still what gets injected. Adds a config-resolution assertion to the start-ssr vitest suite that covers both false-positive paths (plugin-local resolution and NODE_PATH). Co-authored-by: Cursor <cursoragent@cursor.com>
🦋 Changeset detectedLatest commit: bdcc964 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # examples/start-ssr/test/run.mjs
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
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.
Forward-ports two open
main-line fixes onto the@solidjs/vite-plugin3.0.0-next line. Two patch changesets, one per fix.1. jest-dom setup file resolved from the project root — port of #364 (@brenelz), fixes #231
getJestDomExportprobed@testing-library/jest-dom/vitestwith the plugin's ownrequire(fromimport.meta.url). Under pnpm's isolatednode_modulesthat succeeds whenever any package in the tree depends on jest-dom transitively (Storybook in the issue), while Vitest resolves baresetupFilesfrom the project root, where it isn't installed →Failed to load url .../@testing-library/jest-dom/vitest. Reproduced onnextwith a packed tarball (see below).The
next-specific posture logic around the injection (server-posture skip, browser-mode skip) is unchanged; the probe now takes the Vite root (userConfig.root, falling back toprocess.cwd()), as in #364.Divergence from #364: #364 uses
createRequire(path.join(root, 'package.json')).resolve(...). That still gives a false positive on the most common command path:require.resolvealso consultsNODE_PATH, and pnpm's bin shims (pnpm vitest/pnpm test) exportNODE_PATH=…/node_modules/.pnpm/node_modules— the hoisted virtual store where every transitive dep is reachable. Vite's resolver ignoresNODE_PATH, so the injected setup file still failed to load under the shim (verified: the faithful port passed a plainnodeprobe butpnpm vitest runstill failed). The port therefore walks<dir>/node_modules/@testing-library/jest-domup from the root (what Vite/Node do withoutNODE_PATH), then checks the subpath (/vitest, then v5's/extend-expect) against that exact copy. The bare specifier is still what gets injected, for the reason documented in #364. Worth back-porting tomainas well.2. No jsdom default when
test.projects/test.workspaceis set — port of #323 (@carloitaben), fixes #205A root config that defines
test.projects(or the pre-Vitest-4test.workspace) runs no tests itself; each project controls its own environment. Injectingenvironment: 'jsdom'at the root made vitest probe for (and prompt to install) jsdom on startup even when every project runs under node or in browser mode. Same one-line guard as #323, folded intonext's existing browser-mode guard.Note (same on
mainwith #323): an inline project withextends: trueinherits the root file'stest.projects, so such a project also opts out of the jsdom default unless it declaresenvironmentitself — matching vitest's projects guide, where projects declare their environment explicitly. Documented in the code comment.Also checked
nextas 3257a97 —optimizeDeps.rolldownOptions.transform.jsx.runtime: 'classic'is present insrc/index.ts. Nothing to do.$ServerOnlyHMR,omitServerOnlyTemplates: false): not portable.@solidjs/babel-plugin@2.0.0-rc.8hardcodesskipTemplate = truefor$ServerOnlyunderhydratableand neither it nor@solidjs/compiler'sTransformOptionsexpose anomitServerOnlyTemplatesoption. Nothing to do here; it would need a compiler-side option first.Tests
examples/start-ssr/test/run.mjs(vitest mode), both at config-resolution level:projects-env: root config withtest.projectsresolves with notest.environment.jest-dom-root: an empty project outside the repo, resolved in a child process whoseNODE_PATHpoints at the repo'snode_modules(simulating both the plugin-local resolution and the pnpm shim), gets nosetupFiles; the example root (whose ancestors hold the repo's jest-dom) still gets@testing-library/jest-dom/vitest.environment: jsdom/["@testing-library/jest-dom/vitest"]for the empty root) and pass with it.pnpm build,tsc --noEmit,examples/vite-8vitest (browser mode) and the rootpnpm testcypress run pass.file:dep, default jsdom environment:pnpm vitest runfailed withCannot find module '.../@testing-library/jest-dom/vitest'→ passes with no setup file injected. With jest-dom as a direct dep (v6 and v5) the setup file is still injected andtoBeInTheDocumentworks (v6).test.projectswith anodeproject and a project with no environment: roottest.environmentjsdom→undefined; the node project runs under node in both cases; the environment-less project now runs under node instead of failing on the missing jsdom package.