Skip to content

Commit 62cbb17

Browse files
committed
fix: tag warm-modules snapshot with moduleType for injectCjsGlobals
The `fetchWarmModules` fast path serves inline modules to fresh workers without the `moduleType` tag the direct-fetch path attaches (#10709). With `injectCjsGlobals: false` the evaluator injects the CommonJS scope only into modules tagged `moduleType: 'cjs'`, so a CommonJS dependency read back from the snapshot evaluated without `require`/`module`/ `__dirname` and threw "require is not defined" — the first file worked (direct fetch) while every later file served from the snapshot failed. Recompute the tag in `fetchWarmModules` with the same `detectModuleType` the direct-fetch path uses, gated on `injectCjsGlobals === false` so the default path pays nothing.
1 parent 9a42cc7 commit 62cbb17

2 files changed

Lines changed: 67 additions & 0 deletions

File tree

packages/vitest/src/node/pools/rpc.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@ import type { RuntimeRPC } from '../../types/rpc'
44
import type { TestProject } from '../project'
55
import type { ResolveSnapshotPathHandlerContext } from '../types/config'
66
import { existsSync, mkdirSync } from 'node:fs'
7+
import { readFile } from 'node:fs/promises'
78
import { fileURLToPath } from 'node:url'
89
import { cleanUrl } from '@vitest/utils/helpers'
910
import { isBuiltin, toBuiltin } from '../../utils/modules'
1011
import { handleRollupError } from '../environments/fetchModule'
1112
import { normalizeResolvedIdToUrl } from '../environments/normalizeUrl'
13+
import { detectModuleType } from '../resolver'
1214

1315
interface MethodsOptions {
1416
cacheFs?: boolean
@@ -26,6 +28,16 @@ interface MethodsOptions {
2628
// together with the resolver that produced them.
2729
const warmExternals = new WeakMap<ViteDevServer, Record<string, FetchResult>>()
2830

31+
// `detectModuleType` only reads the original source in the ambiguous case where
32+
// the transformed code both looks like ESM and mentions a CommonJS variable;
33+
// mirror the guard `ModuleFetcher.sourceLoader` uses for virtual modules
34+
function warmSourceLoader(file: string | null): (() => Promise<string | null>) | undefined {
35+
if (!file || file.startsWith('\x00') || file.startsWith('virtual:')) {
36+
return undefined
37+
}
38+
return () => readFile(file, 'utf-8').then(source => source, () => null)
39+
}
40+
2941
export function createMethodsRPC(project: TestProject, methodsOptions: MethodsOptions = {}): RuntimeRPC {
3042
const vitest = project.vitest
3143
const cacheFs = methodsOptions.cacheFs ?? false
@@ -84,6 +96,14 @@ export function createMethodsRPC(project: TestProject, methodsOptions: MethodsOp
8496
throw new Error(`The environment ${environmentName} was not defined in the Vite config.`)
8597
}
8698

99+
// with `injectCjsGlobals: false` the evaluator injects the CommonJS
100+
// variables only into modules the server tagged `moduleType: 'cjs'`. that
101+
// tag is produced by the per-module `fetch`, which the warm snapshot
102+
// bypasses, so it has to be recomputed here — otherwise a CommonJS module
103+
// served from the snapshot evaluates without `require`/`module`/`__dirname`
104+
// and throws. the detection is skipped entirely on the default path.
105+
const detectType = project.config.injectCjsGlobals === false
106+
87107
const warm: Record<string, FetchResult | FetchCachedFileSystemResult> = Object.create(null)
88108

89109
// walk the import graphs of the requested files instead of dumping the
@@ -126,6 +146,9 @@ export function createMethodsRPC(project: TestProject, methodsOptions: MethodsOp
126146
tmp,
127147
url: node.url,
128148
invalidate: false,
149+
moduleType: detectType
150+
? await detectModuleType(node.file, transformResult.code, warmSourceLoader(node.file))
151+
: undefined,
129152
}
130153
warm[node.url] = entry
131154
if (node.id !== node.url) {

test/e2e/test/config/injectCjsGlobals.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,50 @@ test.for(pools)('inlined ".cjs" modules keep the module scope when injectCjsGlob
6363
expect(exitCode).toBe(0)
6464
})
6565

66+
test('cjs dep served from the warm-modules snapshot keeps the module scope when injectCjsGlobals is disabled', async () => {
67+
// the `fetchWarmModules` fast path hands a module the server already
68+
// transformed to a fresh worker without the per-module `fetch` that tags its
69+
// `moduleType`. that tag is what tells the evaluator to inject the CommonJS
70+
// scope when `injectCjsGlobals` is disabled, so the snapshot has to carry it.
71+
// isolate + sequential files make the ordering deterministic: the first file
72+
// transforms and caches the dependency (direct fetch), every later file reads
73+
// it back from the snapshot — without the tag those files throw
74+
// "require is not defined" while the first one still passes.
75+
const structure: Record<string, string> = {
76+
'package.json': '{ "type": "module" }',
77+
'cjs-dep.cjs': ts`
78+
const path = require('node:path')
79+
module.exports = {
80+
answer: 42,
81+
base: path.basename(__filename),
82+
dir: typeof __dirname,
83+
}
84+
`,
85+
}
86+
for (const name of ['a', 'b', 'c', 'd']) {
87+
structure[`${name}.test.js`] = ts`
88+
import { expect, test } from 'vitest'
89+
import cjs from './cjs-dep.cjs'
90+
91+
test('cjs module keeps its scope', () => {
92+
expect(cjs.answer).toBe(42)
93+
expect(cjs.base).toBe('cjs-dep.cjs')
94+
expect(cjs.dir).toBe('string')
95+
expect(typeof module).toBe('undefined')
96+
})
97+
`
98+
}
99+
const { stderr, exitCode } = await runInlineTests(structure, {
100+
pool: 'forks',
101+
isolate: true,
102+
fileParallelism: false,
103+
injectCjsGlobals: false,
104+
experimental: { fsModuleCache: true },
105+
})
106+
expect(stderr).toBe('')
107+
expect(exitCode).toBe(0)
108+
})
109+
66110
test('".js" modules without ESM syntax are detected as commonjs in a typeless package', async () => {
67111
const { stderr, exitCode } = await runInlineTests({
68112
'package.json': '{}',

0 commit comments

Comments
 (0)