Skip to content

Commit 084e929

Browse files
sheremet-vasheremet-va
authoredDec 16, 2022
fix!: correctly interop nested default for external and inlined modules (#2512)
* fix: correctly interop nested default for external and inlined modules, if environment is not node * chore: cleanup * chore: cleanup * test: update default interop test * chore: update module interop tests * fix: add environment to enzym example * chore: update tests * test: don't run module tests without threads, because we don't clear ESM cache * chore: correctly reset current test environment
·
v3.1.0-beta.2v0.26.0
1 parent 58ee8e9 commit 084e929

19 files changed

+196
-90
lines changed
 

‎examples/react-enzyme/package.json

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
"@vitest/ui": "latest",
1717
"@wojtekmaj/enzyme-adapter-react-17": "^0.6.7",
1818
"enzyme": "^3.11.0",
19+
"jsdom": "^20.0.3",
1920
"vite": "latest",
2021
"vitest": "latest"
2122
},

‎examples/react-enzyme/vite.config.ts

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { defineConfig } from 'vitest/config'
66
export default defineConfig({
77
plugins: [react()],
88
test: {
9+
environment: 'jsdom',
910
setupFiles: ['./vitest.setup.ts'],
1011
},
1112
})

‎packages/vite-node/src/client.ts

+43-25
Original file line numberDiff line numberDiff line change
@@ -273,9 +273,17 @@ export class ViteNodeRunner {
273273
enumerable: false,
274274
configurable: false,
275275
})
276-
// this prosxy is triggered only on exports.name and module.exports access
276+
// this prosxy is triggered only on exports.{name} and module.exports access
277277
const cjsExports = new Proxy(exports, {
278-
set(_, p, value) {
278+
set: (_, p, value) => {
279+
// treat "module.exports =" the same as "exports.default =" to not have nested "default.default",
280+
// so "exports.default" becomes the actual module
281+
if (p === 'default' && this.shouldInterop(url, { default: value })) {
282+
exportAll(cjsExports, value)
283+
exports.default = value
284+
return true
285+
}
286+
279287
if (!Reflect.has(exports, 'default'))
280288
exports.default = {}
281289

@@ -378,35 +386,45 @@ export class ViteNodeRunner {
378386
* Import a module and interop it
379387
*/
380388
async interopedImport(path: string) {
381-
const mod = await import(path)
382-
383-
if (this.shouldInterop(path, mod)) {
384-
const tryDefault = this.hasNestedDefault(mod)
385-
return new Proxy(mod, {
386-
get: proxyMethod('get', tryDefault),
387-
set: proxyMethod('set', tryDefault),
388-
has: proxyMethod('has', tryDefault),
389-
deleteProperty: proxyMethod('deleteProperty', tryDefault),
390-
})
391-
}
389+
const importedModule = await import(path)
392390

393-
return mod
394-
}
391+
if (!this.shouldInterop(path, importedModule))
392+
return importedModule
395393

396-
hasNestedDefault(target: any) {
397-
return '__esModule' in target && target.__esModule && 'default' in target.default
394+
const { mod, defaultExport } = interopModule(importedModule)
395+
396+
return new Proxy(mod, {
397+
get(mod, prop) {
398+
if (prop === 'default')
399+
return defaultExport
400+
return mod[prop] ?? defaultExport?.[prop]
401+
},
402+
has(mod, prop) {
403+
if (prop === 'default')
404+
return defaultExport !== undefined
405+
return prop in mod || (defaultExport && prop in defaultExport)
406+
},
407+
})
398408
}
399409
}
400410

401-
function proxyMethod(name: 'get' | 'set' | 'has' | 'deleteProperty', tryDefault: boolean) {
402-
return function (target: any, key: string | symbol, ...args: [any?, any?]): any {
403-
const result = Reflect[name](target, key, ...args)
404-
if (isPrimitive(target.default))
405-
return result
406-
if ((tryDefault && key === 'default') || typeof result === 'undefined')
407-
return Reflect[name](target.default, key, ...args)
408-
return result
411+
function interopModule(mod: any) {
412+
if (isPrimitive(mod)) {
413+
return {
414+
mod: { default: mod },
415+
defaultExport: mod,
416+
}
409417
}
418+
419+
let defaultExport = 'default' in mod ? mod.default : mod
420+
421+
if (!isPrimitive(defaultExport) && '__esModule' in defaultExport) {
422+
mod = defaultExport
423+
if ('default' in defaultExport)
424+
defaultExport = defaultExport.default
425+
}
426+
427+
return { mod, defaultExport }
410428
}
411429

412430
// keep consistency with Vite on how exports are defined

‎packages/vitest/src/integrations/chai/index.ts

+2-4
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
import * as chai from 'chai'
22
import './setup'
33
import type { Test } from '../../types'
4-
import { getFullName, getWorkerState } from '../../utils'
4+
import { getCurrentEnvironment, getFullName } from '../../utils'
55
import type { MatcherState } from '../../types/chai'
66
import { getState, setState } from './jest-expect'
77
import { GLOBAL_EXPECT } from './constants'
88

9-
const workerState = getWorkerState()
10-
119
export function createExpect(test?: Test) {
1210
const expect = ((value: any, message?: string): Vi.Assertion => {
1311
const { assertionCalls } = getState(expect)
@@ -30,7 +28,7 @@ export function createExpect(test?: Test) {
3028
isExpectingAssertionsError: null,
3129
expectedAssertionsNumber: null,
3230
expectedAssertionsNumberErrorGen: null,
33-
environment: workerState.config.environment,
31+
environment: getCurrentEnvironment(),
3432
testPath: test?.suite.file?.filepath,
3533
currentTestName: test ? getFullName(test) : undefined,
3634
}, expect)

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

+6-3
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ export async function run(files: string[], config: ResolvedConfig): Promise<void
5353
if (!files || !files.length)
5454
continue
5555

56+
// @ts-expect-error untyped global
57+
globalThis.__vitest_environment__ = environment
58+
5659
const filesByOptions = groupBy(files, ({ envOptions }) => JSON.stringify(envOptions))
5760

5861
for (const options of Object.keys(filesByOptions)) {
@@ -63,9 +66,9 @@ export async function run(files: string[], config: ResolvedConfig): Promise<void
6366

6467
await withEnv(environment, files[0].envOptions || config.environmentOptions || {}, async () => {
6568
for (const { file } of files) {
66-
// it doesn't matter if running with --threads
67-
// if running with --no-threads, we usually want to reset everything before running a test
68-
// but we have --isolate option to disable this
69+
// it doesn't matter if running with --threads
70+
// if running with --no-threads, we usually want to reset everything before running a test
71+
// but we have --isolate option to disable this
6972
if (config.isolate) {
7073
workerState.mockMap.clear()
7174
resetModules(workerState.moduleCache, true)

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

+5-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { ViteNodeRunner } from 'vite-node/client'
22
import type { ViteNodeRunnerOptions } from 'vite-node'
33
import { normalizePath } from 'vite'
44
import type { MockMap } from '../types/mocker'
5-
import { getWorkerState } from '../utils'
5+
import { getCurrentEnvironment, getWorkerState } from '../utils'
66
import { VitestMocker } from './mocker'
77

88
export interface ExecuteOptions extends ViteNodeRunnerOptions {
@@ -60,4 +60,8 @@ export class VitestRunner extends ViteNodeRunner {
6060
__vitest_mocker__: this.mocker,
6161
})
6262
}
63+
64+
shouldInterop(path: string, mod: any) {
65+
return this.options.interopDefault ?? (getCurrentEnvironment() !== 'node' && super.shouldInterop(path, mod))
66+
}
6367
}

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

+3-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ async function startViteNode(ctx: WorkerContext) {
5050
},
5151
moduleCache,
5252
mockMap,
53-
interopDefault: config.deps.interopDefault ?? true,
53+
interopDefault: config.deps.interopDefault,
5454
root: config.root,
5555
base: config.base,
5656
}))[0]
@@ -70,6 +70,8 @@ function init(ctx: WorkerContext) {
7070
process.env.VITEST_WORKER_ID = String(workerId)
7171
process.env.VITEST_POOL_ID = String(poolId)
7272

73+
// @ts-expect-error untyped global
74+
globalThis.__vitest_environment__ = config.environment
7375
// @ts-expect-error I know what I am doing :P
7476
globalThis.__vitest_worker__ = {
7577
ctx,

‎packages/vitest/src/utils/global.ts

+5
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,8 @@ export function getWorkerState(): WorkerGlobalState {
44
// @ts-expect-error untyped global
55
return globalThis.__vitest_worker__
66
}
7+
8+
export function getCurrentEnvironment(): string {
9+
// @ts-expect-error untyped global
10+
return globalThis.__vitest_environment__
11+
}

‎packages/vitest/src/utils/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ export function resetModules(modules: ModuleCacheMap, resetMocks = false) {
4949
const skipPaths = [
5050
// Vitest
5151
/\/vitest\/dist\//,
52+
/\/vite-node\/dist\//,
5253
// yarn's .store folder
5354
/vitest-virtual-\w+\/dist/,
5455
// cnpm

‎pnpm-lock.yaml

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

‎test/cjs/test/function-default.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,6 @@ describe('correctly puts default on default', () => {
88

99
it('works on nested default function', () => {
1010
// @ts-expect-error types defined only default
11-
expect(format.default()).toBe('')
11+
expect(format.default).toBeUndefined()
1212
})
1313
})

‎test/cjs/test/named-default.test.ts

-15
This file was deleted.

‎test/cjs/vitest.config.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1-
import { defineConfig } from 'vite'
1+
import { defineConfig } from 'vitest/config'
22

33
export default defineConfig({
44
test: {
5-
// threads: false,
5+
deps: {
6+
interopDefault: true,
7+
},
68
},
79
})

‎test/core/src/cjs/nested-default-cjs.js

+8
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 })

‎test/core/src/external/default-cjs.js

+7
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+
}

‎test/core/src/external/nested-default-cjs.js

+8
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 })

‎test/core/test/module-dom.test.ts

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
5+
import { describe, expect, it } from 'vitest'
6+
// @ts-expect-error is not typed with imports
7+
import * as nestedDefaultCjs from '../src/cjs/nested-default-cjs'
8+
// @ts-expect-error is not typed with imports
9+
import * as nestedDefaultExternalCjs from '../src/external/nested-default-cjs'
10+
// @ts-expect-error is not typed with imports
11+
import * as moduleDefaultCjs from '../src/external/default-cjs'
12+
13+
describe('validating nested defaults in isolation', () => {
14+
it.each([
15+
nestedDefaultCjs,
16+
nestedDefaultExternalCjs,
17+
])('nested default should be resolved, because environment is not node', (mod) => {
18+
expect(mod).toHaveProperty('default')
19+
expect(mod.default).not.toHaveProperty('default')
20+
expect(mod.default.a).toBe('a')
21+
expect(mod.default.b).toBe('b')
22+
expect(mod.a).toBe('a')
23+
expect(mod.b).toBe('b')
24+
})
25+
26+
it('externalized "module.exports" CJS module interops default', () => {
27+
expect(moduleDefaultCjs).toHaveProperty('default')
28+
expect(moduleDefaultCjs.default).toHaveProperty('a')
29+
expect(moduleDefaultCjs.default.a).toBe('a')
30+
expect(moduleDefaultCjs).toHaveProperty('a')
31+
expect(moduleDefaultCjs.a).toBe('a')
32+
})
33+
})

‎test/core/test/module.test.ts

+25-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { expect, it } from 'vitest'
1+
import { describe, expect, it } from 'vitest'
22
// @ts-expect-error is not typed
33
import cjs, { a, b } from '../src/cjs/module-cjs'
44
// @ts-expect-error is not typed with imports
@@ -12,6 +12,12 @@ import * as arrayCjs from '../src/cjs/array-cjs'
1212
// @ts-expect-error is not typed with imports
1313
import * as classCjs from '../src/cjs/class-cjs'
1414
// @ts-expect-error is not typed with imports
15+
import * as nestedDefaultCjs from '../src/cjs/nested-default-cjs'
16+
// @ts-expect-error is not typed with imports
17+
import * as nestedDefaultExternalCjs from '../src/external/nested-default-cjs'
18+
// @ts-expect-error is not typed with imports
19+
import * as moduleDefaultCjs from '../src/external/default-cjs'
20+
// @ts-expect-error is not typed with imports
1521
import * as internalEsm from '../src/esm/internal-esm.mjs'
1622
import c, { d } from '../src/module-esm'
1723
import * as timeout from '../src/timeout'
@@ -20,6 +26,24 @@ it('doesn\'t when extending module', () => {
2026
expect(() => Object.assign(globalThis, timeout)).not.toThrow()
2127
})
2228

29+
describe('validating nested defaults in isolation', () => {
30+
it.each([
31+
nestedDefaultCjs,
32+
nestedDefaultExternalCjs,
33+
])('nested default should stay, because environment is node', (mod) => {
34+
expect(mod).toHaveProperty('default')
35+
expect(mod.default).toHaveProperty('default')
36+
expect(mod.default.default.a).toBe('a')
37+
expect(mod.default.default.b).toBe('b')
38+
})
39+
40+
it('don\'t interop external module.exports, because environment is node', () => {
41+
expect(moduleDefaultCjs).toHaveProperty('default')
42+
expect(moduleDefaultCjs.default).toHaveProperty('a')
43+
expect(moduleDefaultCjs).not.toHaveProperty('a')
44+
})
45+
})
46+
2347
it('should work when using module.exports cjs', () => {
2448
expect(cjs.a).toBe(1)
2549
expect(cjs.b).toBe(2)

‎test/core/vitest.config.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export default defineConfig({
6060
seed: 101,
6161
},
6262
deps: {
63-
external: ['tinyspy'],
63+
external: ['tinyspy', /src\/external/],
6464
},
6565
alias: [
6666
{

0 commit comments

Comments
 (0)
Please sign in to comment.