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

Author: sheremet-va

examples/react-enzyme/package.json

@@ -16,6 +16,7 @@
     "@vitest/ui": "latest",
     "@wojtekmaj/enzyme-adapter-react-17": "^0.6.7",
     "enzyme": "^3.11.0",
+    "jsdom": "^20.0.3",
     "vite": "latest",
     "vitest": "latest"
   },

examples/react-enzyme/vite.config.ts

@@ -6,6 +6,7 @@ import { defineConfig } from 'vitest/config'
 export default defineConfig({
   plugins: [react()],
   test: {
+    environment: 'jsdom',
     setupFiles: ['./vitest.setup.ts'],
   },
 })

packages/vite-node/src/client.ts

@@ -273,9 +273,17 @@ export class ViteNodeRunner {
       enumerable: false,
       configurable: false,
     })
-    // this prosxy is triggered only on exports.name and module.exports access
+    // this prosxy is triggered only on exports.{name} and module.exports access
     const cjsExports = new Proxy(exports, {
-      set(_, p, value) {
+      set: (_, p, value) => {
+        // treat "module.exports =" the same as "exports.default =" to not have nested "default.default",
+        // so "exports.default" becomes the actual module
+        if (p === 'default' && this.shouldInterop(url, { default: value })) {
+          exportAll(cjsExports, value)
+          exports.default = value
+          return true
+        }
+
         if (!Reflect.has(exports, 'default'))
           exports.default = {}
 
@@ -378,35 +386,45 @@ export class ViteNodeRunner {
    * Import a module and interop it
    */
   async interopedImport(path: string) {
-    const mod = await import(path)
-
-    if (this.shouldInterop(path, mod)) {
-      const tryDefault = this.hasNestedDefault(mod)
-      return new Proxy(mod, {
-        get: proxyMethod('get', tryDefault),
-        set: proxyMethod('set', tryDefault),
-        has: proxyMethod('has', tryDefault),
-        deleteProperty: proxyMethod('deleteProperty', tryDefault),
-      })
-    }
+    const importedModule = await import(path)
 
-    return mod
-  }
+    if (!this.shouldInterop(path, importedModule))
+      return importedModule
 
-  hasNestedDefault(target: any) {
-    return '__esModule' in target && target.__esModule && 'default' in target.default
+    const { mod, defaultExport } = interopModule(importedModule)
+
+    return new Proxy(mod, {
+      get(mod, prop) {
+        if (prop === 'default')
+          return defaultExport
+        return mod[prop] ?? defaultExport?.[prop]
+      },
+      has(mod, prop) {
+        if (prop === 'default')
+          return defaultExport !== undefined
+        return prop in mod || (defaultExport && prop in defaultExport)
+      },
+    })
   }
 }
 
-function proxyMethod(name: 'get' | 'set' | 'has' | 'deleteProperty', tryDefault: boolean) {
-  return function (target: any, key: string | symbol, ...args: [any?, any?]): any {
-    const result = Reflect[name](target, key, ...args)
-    if (isPrimitive(target.default))
-      return result
-    if ((tryDefault && key === 'default') || typeof result === 'undefined')
-      return Reflect[name](target.default, key, ...args)
-    return result
+function interopModule(mod: any) {
+  if (isPrimitive(mod)) {
+    return {
+      mod: { default: mod },
+      defaultExport: mod,
+    }
   }
+
+  let defaultExport = 'default' in mod ? mod.default : mod
+
+  if (!isPrimitive(defaultExport) && '__esModule' in defaultExport) {
+    mod = defaultExport
+    if ('default' in defaultExport)
+      defaultExport = defaultExport.default
+  }
+
+  return { mod, defaultExport }
 }
 
 // keep consistency with Vite on how exports are defined

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

@@ -1,13 +1,11 @@
 import * as chai from 'chai'
 import './setup'
 import type { Test } from '../../types'
-import { getFullName, getWorkerState } from '../../utils'
+import { getCurrentEnvironment, getFullName } from '../../utils'
 import type { MatcherState } from '../../types/chai'
 import { getState, setState } from './jest-expect'
 import { GLOBAL_EXPECT } from './constants'
 
-const workerState = getWorkerState()
-
 export function createExpect(test?: Test) {
   const expect = ((value: any, message?: string): Vi.Assertion => {
     const { assertionCalls } = getState(expect)
@@ -30,7 +28,7 @@ export function createExpect(test?: Test) {
     isExpectingAssertionsError: null,
     expectedAssertionsNumber: null,
     expectedAssertionsNumberErrorGen: null,
-    environment: workerState.config.environment,
+    environment: getCurrentEnvironment(),
     testPath: test?.suite.file?.filepath,
     currentTestName: test ? getFullName(test) : undefined,
   }, expect)

packages/vitest/src/runtime/entry.ts

@@ -53,6 +53,9 @@ export async function run(files: string[], config: ResolvedConfig): Promise<void
     if (!files || !files.length)
       continue
 
+    // @ts-expect-error untyped global
+    globalThis.__vitest_environment__ = environment
+
     const filesByOptions = groupBy(files, ({ envOptions }) => JSON.stringify(envOptions))
 
     for (const options of Object.keys(filesByOptions)) {
@@ -63,9 +66,9 @@ export async function run(files: string[], config: ResolvedConfig): Promise<void
 
       await withEnv(environment, files[0].envOptions || config.environmentOptions || {}, async () => {
         for (const { file } of files) {
-        // it doesn't matter if running with --threads
-        // if running with --no-threads, we usually want to reset everything before running a test
-        // but we have --isolate option to disable this
+          // it doesn't matter if running with --threads
+          // if running with --no-threads, we usually want to reset everything before running a test
+          // but we have --isolate option to disable this
           if (config.isolate) {
             workerState.mockMap.clear()
             resetModules(workerState.moduleCache, true)

packages/vitest/src/runtime/execute.ts

@@ -2,7 +2,7 @@ import { ViteNodeRunner } from 'vite-node/client'
 import type { ViteNodeRunnerOptions } from 'vite-node'
 import { normalizePath } from 'vite'
 import type { MockMap } from '../types/mocker'
-import { getWorkerState } from '../utils'
+import { getCurrentEnvironment, getWorkerState } from '../utils'
 import { VitestMocker } from './mocker'
 
 export interface ExecuteOptions extends ViteNodeRunnerOptions {
@@ -60,4 +60,8 @@ export class VitestRunner extends ViteNodeRunner {
       __vitest_mocker__: this.mocker,
     })
   }
+
+  shouldInterop(path: string, mod: any) {
+    return this.options.interopDefault ?? (getCurrentEnvironment() !== 'node' && super.shouldInterop(path, mod))
+  }
 }

packages/vitest/src/runtime/worker.ts

@@ -50,7 +50,7 @@ async function startViteNode(ctx: WorkerContext) {
     },
     moduleCache,
     mockMap,
-    interopDefault: config.deps.interopDefault ?? true,
+    interopDefault: config.deps.interopDefault,
     root: config.root,
     base: config.base,
   }))[0]
@@ -70,6 +70,8 @@ function init(ctx: WorkerContext) {
   process.env.VITEST_WORKER_ID = String(workerId)
   process.env.VITEST_POOL_ID = String(poolId)
 
+  // @ts-expect-error untyped global
+  globalThis.__vitest_environment__ = config.environment
   // @ts-expect-error I know what I am doing :P
   globalThis.__vitest_worker__ = {
     ctx,

packages/vitest/src/utils/global.ts

@@ -4,3 +4,8 @@ export function getWorkerState(): WorkerGlobalState {
   // @ts-expect-error untyped global
   return globalThis.__vitest_worker__
 }
+
+export function getCurrentEnvironment(): string {
+  // @ts-expect-error untyped global
+  return globalThis.__vitest_environment__
+}

packages/vitest/src/utils/index.ts

@@ -49,6 +49,7 @@ export function resetModules(modules: ModuleCacheMap, resetMocks = false) {
   const skipPaths = [
     // Vitest
     /\/vitest\/dist\//,
+    /\/vite-node\/dist\//,
     // yarn's .store folder
     /vitest-virtual-\w+\/dist/,
     // cnpm