Skip to content

Commit 0c53e09

Browse files
authoredAug 3, 2023
fix: don't crash when using --experimental-vm-threads, interop CJS default inside node_modules (#3876)
1 parent 3e1e7a1 commit 0c53e09

15 files changed

+792
-630
lines changed
 

‎packages/vitest/src/runtime/execute.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,11 @@ export class VitestExecutor extends ViteNodeRunner {
146146
}
147147

148148
constructor(public options: ExecuteOptions) {
149-
super(options)
149+
super({
150+
...options,
151+
// interop is done inside the external executor instead
152+
interopDefault: options.context ? false : options.interopDefault,
153+
})
150154

151155
this.mocker = new VitestMocker(this)
152156

@@ -169,6 +173,7 @@ export class VitestExecutor extends ViteNodeRunner {
169173
}
170174
else {
171175
this.externalModules = new ExternalModulesExecutor({
176+
...options,
172177
context: options.context,
173178
packageCache: options.packageCache,
174179
})

‎packages/vitest/src/runtime/external-executor.ts

+56-10
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@ import { dirname } from 'node:path'
66
import { Module as _Module, createRequire } from 'node:module'
77
import { readFileSync, statSync } from 'node:fs'
88
import { basename, extname, join, normalize } from 'pathe'
9-
import { getCachedData, isNodeBuiltin, setCacheData } from 'vite-node/utils'
9+
import { getCachedData, isNodeBuiltin, isPrimitive, setCacheData } from 'vite-node/utils'
1010
import { CSS_LANGS_RE, KNOWN_ASSET_RE } from 'vite-node/constants'
1111
import { getColors } from '@vitest/utils'
12+
import type { ExecuteOptions } from './execute'
1213

1314
// need to copy paste types for vm
1415
// because they require latest @types/node which we don't bundle
@@ -95,7 +96,7 @@ const nativeResolve = import.meta.resolve!
9596
const dataURIRegex
9697
= /^data:(?<mime>text\/javascript|application\/json|application\/wasm)(?:;(?<encoding>charset=utf-8|base64))?,(?<code>.*)$/
9798

98-
interface ExternalModulesExecutorOptions {
99+
export interface ExternalModulesExecutorOptions extends ExecuteOptions {
99100
context: vm.Context
100101
packageCache: Map<string, any>
101102
}
@@ -273,7 +274,7 @@ export class ExternalModulesExecutor {
273274
return buffer
274275
}
275276

276-
private findNearestPackageData(basedir: string) {
277+
private findNearestPackageData(basedir: string): { type?: 'module' | 'commonjs' } {
277278
const originalBasedir = basedir
278279
const packageCache = this.options.packageCache
279280
while (basedir) {
@@ -300,12 +301,11 @@ export class ExternalModulesExecutor {
300301
basedir = nextBasedir
301302
}
302303

303-
return null
304+
return {}
304305
}
305306

306-
private wrapSynteticModule(identifier: string, exports: Record<string, unknown>) {
307-
// TODO: technically module should be parsed to find static exports, implement for strict mode in #2854
308-
const moduleKeys = Object.keys(exports).filter(key => key !== 'default')
307+
private wrapCoreSynteticModule(identifier: string, exports: Record<string, unknown>) {
308+
const moduleKeys = Object.keys(exports)
309309
const m: any = new SyntheticModule(
310310
[...moduleKeys, 'default'],
311311
() => {
@@ -321,6 +321,52 @@ export class ExternalModulesExecutor {
321321
return m
322322
}
323323

324+
private interopCommonJsModule(mod: any) {
325+
if (isPrimitive(mod) || Array.isArray(mod) || mod instanceof Promise) {
326+
return {
327+
keys: [],
328+
moduleExports: {},
329+
defaultExport: mod,
330+
}
331+
}
332+
333+
if (this.options.interopDefault !== false && '__esModule' in mod && !isPrimitive(mod.default)) {
334+
return {
335+
keys: Array.from(new Set(Object.keys(mod.default).concat(Object.keys(mod)).filter(key => key !== 'default'))),
336+
moduleExports: new Proxy(mod, {
337+
get(mod, prop) {
338+
return mod[prop] ?? mod.default?.[prop]
339+
},
340+
}),
341+
defaultExport: mod,
342+
}
343+
}
344+
345+
return {
346+
keys: Object.keys(mod).filter(key => key !== 'default'),
347+
moduleExports: mod,
348+
defaultExport: mod,
349+
}
350+
}
351+
352+
private wrapCommonJsSynteticModule(identifier: string, exports: Record<string, unknown>) {
353+
// TODO: technically module should be parsed to find static exports, implement for strict mode in #2854
354+
const { keys, moduleExports, defaultExport } = this.interopCommonJsModule(exports)
355+
const m: any = new SyntheticModule(
356+
[...keys, 'default'],
357+
() => {
358+
for (const key of keys)
359+
m.setExport(key, moduleExports[key])
360+
m.setExport('default', defaultExport)
361+
},
362+
{
363+
context: this.context,
364+
identifier,
365+
},
366+
)
367+
return m
368+
}
369+
324370
private async evaluateModule<T extends VMModule>(m: T): Promise<T> {
325371
if (m.status === 'unlinked') {
326372
this.esmLinkMap.set(
@@ -582,7 +628,7 @@ c.green(`export default {
582628

583629
if (extension === '.node' || isNodeBuiltin(identifier)) {
584630
const exports = this.requireCoreModule(identifier)
585-
return this.wrapSynteticModule(identifier, exports)
631+
return this.wrapCoreSynteticModule(identifier, exports)
586632
}
587633

588634
const isFileUrl = identifier.startsWith('file://')
@@ -600,7 +646,7 @@ c.green(`export default {
600646
if (extension === '.cjs') {
601647
const module = this.createCommonJSNodeModule(pathUrl)
602648
const exports = this.loadCommonJSModule(module, pathUrl)
603-
return this.wrapSynteticModule(fileUrl, exports)
649+
return this.wrapCommonJsSynteticModule(fileUrl, exports)
604650
}
605651

606652
if (extension === '.mjs')
@@ -613,7 +659,7 @@ c.green(`export default {
613659

614660
const module = this.createCommonJSNodeModule(pathUrl)
615661
const exports = this.loadCommonJSModule(module, pathUrl)
616-
return this.wrapSynteticModule(fileUrl, exports)
662+
return this.wrapCommonJsSynteticModule(fileUrl, exports)
617663
}
618664

619665
async import(identifier: string) {

‎pnpm-lock.yaml

+643-618
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/test-utils/index.ts

+10-1
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import { Console } from 'node:console'
22
import { Writable } from 'node:stream'
33
import fs from 'node:fs'
4+
import { fileURLToPath } from 'node:url'
45
import { type UserConfig, type VitestRunMode, afterEach } from 'vitest'
56
import type { Vitest } from 'vitest/node'
67
import { startVitest } from 'vitest/node'
78
import { type Options, execa } from 'execa'
89
import stripAnsi from 'strip-ansi'
10+
import { dirname, resolve } from 'pathe'
911

1012
export async function runVitest(config: UserConfig, cliFilters: string[] = [], mode: VitestRunMode = 'test') {
1113
// Reset possible previous runs
@@ -215,14 +217,16 @@ afterEach(() => {
215217
fs.writeFileSync(file, content, 'utf-8')
216218
})
217219
createdFiles.forEach((file) => {
218-
fs.unlinkSync(file)
220+
if (fs.existsSync(file))
221+
fs.unlinkSync(file)
219222
})
220223
originalFiles.clear()
221224
createdFiles.clear()
222225
})
223226

224227
export function createFile(file: string, content: string) {
225228
createdFiles.add(file)
229+
fs.mkdirSync(dirname(file), { recursive: true })
226230
fs.writeFileSync(file, content, 'utf-8')
227231
}
228232

@@ -232,3 +236,8 @@ export function editFile(file: string, callback: (content: string) => string) {
232236
originalFiles.set(file, content)
233237
fs.writeFileSync(file, callback(content), 'utf-8')
234238
}
239+
240+
export function resolvePath(baseUrl: string, path: string) {
241+
const filename = fileURLToPath(baseUrl)
242+
return resolve(dirname(filename), path)
243+
}

‎test/test-utils/package.json

+1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
{
22
"name": "@vitest/test-utils",
33
"private": true,
4+
"main": "./index.ts",
45
"scripts": {
56
"test": "echo \"No tests\""
67
},

‎test/vm-threads/package.json

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"name": "@vitest/test-vm-threads",
3+
"private": true,
4+
"scripts": {
5+
"test": "vitest"
6+
},
7+
"devDependencies": {
8+
"vitest": "workspace:*"
9+
}
10+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
module.exports = {
2+
a: 'a',
3+
b: 'b',
4+
object: {
5+
h: 'h',
6+
},
7+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export * from './default-cjs.cjs'
2+
export { default } from './default-cjs.cjs'
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export * from './nested-default-cjs.cjs'
2+
export { default } from './nested-default-cjs.cjs'
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
exports.default = {
2+
a: 'a',
3+
b: 'b',
4+
object: {
5+
h: 'h',
6+
},
7+
}
8+
Object.defineProperty(exports, '__esModule', { value: true, enumerable: false })
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
module.exports = 42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{ "type": "module" }
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
module.exports = 'string'
+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { createRequire } from 'node:module'
2+
import { describe, expect, test } from 'vitest'
3+
import { createFile, resolvePath } from '../../test-utils'
4+
5+
const require = createRequire(import.meta.url)
6+
7+
test('can require if package.json is null', () => {
8+
createFile(resolvePath(import.meta.url, '../src/external/package-null/package-null.json'), 'null')
9+
expect(() => require('../src/external/package-null/package-null.js')).not.toThrow()
10+
})
11+
12+
describe('validating nested defaults in isolation', async () => {
13+
const nestedDefaultExternalCjs = await import('../src/external/export-nested-default-cjs.js')
14+
const moduleDefaultCjs = await import('../src/external/export-default-cjs.js')
15+
16+
test('nested default should be resolved', () => {
17+
expect(nestedDefaultExternalCjs).toHaveProperty('default')
18+
expect(nestedDefaultExternalCjs.default).not.toHaveProperty('default')
19+
expect(nestedDefaultExternalCjs.default.a).toBe('a')
20+
expect(nestedDefaultExternalCjs.default.b).toBe('b')
21+
expect(nestedDefaultExternalCjs.a).toBe('a')
22+
expect(nestedDefaultExternalCjs.b).toBe('b')
23+
})
24+
25+
test('externalized "module.exports" CJS module interops default', () => {
26+
expect(moduleDefaultCjs).toHaveProperty('default')
27+
expect(moduleDefaultCjs.default).toHaveProperty('a')
28+
expect(moduleDefaultCjs.default.a).toBe('a')
29+
expect(moduleDefaultCjs).toHaveProperty('a')
30+
expect(moduleDefaultCjs.a).toBe('a')
31+
})
32+
})

‎test/vm-threads/vitest.config.ts

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { defineConfig } from 'vitest/config'
2+
3+
export default defineConfig({
4+
test: {
5+
experimentalVmThreads: true,
6+
server: {
7+
deps: {
8+
external: [/src\/external/],
9+
},
10+
},
11+
},
12+
})

0 commit comments

Comments
 (0)
Please sign in to comment.