feat!: improve interchangeability with cjs (vitest-dev#1944)

packages/vite-node/src/client.ts

@@ -4,7 +4,7 @@ import vm from 'vm'
 import { dirname, extname, isAbsolute, resolve } from 'pathe'
 import { isNodeBuiltin } from 'mlly'
 import createDebug from 'debug'
-import { isPrimitive, mergeSlashes, normalizeModuleId, normalizeRequestId, slash, toFilePath } from './utils'
+import { getType, isPrimitive, mergeSlashes, normalizeModuleId, normalizeRequestId, slash, toFilePath } from './utils'
 import type { HotContext, ModuleCache, ViteNodeRunnerOptions } from './types'
 
 const debugExecute = createDebug('vite-node:client:execute')
@@ -241,17 +241,43 @@ export class ViteNodeRunner {
       enumerable: false,
       configurable: false,
     })
+    // this prosxy is triggered only on exports.name and module.exports access
+    const cjsExports = new Proxy(exports, {
+      get(_, p, receiver) {
+        return Reflect.get(exports, p, receiver)
+      },
+      set(_, p, value) {
+        // Node also allows access of named exports via exports.default
+        // https://nodejs.org/api/esm.html#commonjs-namespaces
+        if (p !== 'default') {
+          if (!Reflect.has(exports, 'default'))
+            exports.default = {}
+
+          // returns undefined, when accessing named exports, if default is not an object
+          // but is still present inside hasOwnKeys, this is Node behaviour for CJS
+          if (exports.default === null || typeof exports.default !== 'object') {
+            defineExport(exports, p, () => undefined)
+            return true
+          }
+
+          exports.default[p] = value
+          defineExport(exports, p, () => value)
+          return true
+        }
+        return Reflect.set(exports, p, value)
+      },
+    })
 
     Object.assign(mod, { code: transformed, exports })
 
     const __filename = fileURLToPath(url)
     const moduleProxy = {
       set exports(value) {
-        exportAll(exports, value)
-        exports.default = value
+        exportAll(cjsExports, value)
+        cjsExports.default = value
       },
       get exports() {
-        return exports
+        return cjsExports
       },
     }
 
@@ -282,7 +308,7 @@ export class ViteNodeRunner {
 
       // cjs compact
       require: createRequire(url),
-      exports,
+      exports: cjsExports,
       module: moduleProxy,
       __filename,
       __dirname: dirname(__filename),
@@ -363,20 +389,35 @@ function proxyMethod(name: 'get' | 'set' | 'has' | 'deleteProperty', tryDefault:
   }
 }
 
+// keep consistency with Vite on how exports are defined
+function defineExport(exports: any, key: string | symbol, value: () => any) {
+  Object.defineProperty(exports, key, {
+    enumerable: true,
+    configurable: true,
+    get: value,
+  })
+}
+
 function exportAll(exports: any, sourceModule: any) {
   // #1120 when a module exports itself it causes
   // call stack error
   if (exports === sourceModule)
     return
 
+  const type = getType(sourceModule)
+  if (type !== 'Object' && type !== 'Module')
+    return
+
+  const constructor = sourceModule.constructor?.name
+
+  // allow only plain objects and modules (modules don't have name)
+  if (constructor && constructor !== 'Object')
+    return
+
   for (const key in sourceModule) {
     if (key !== 'default') {
       try {
-        Object.defineProperty(exports, key, {
-          enumerable: true,
-          configurable: true,
-          get() { return sourceModule[key] },
-        })
+        defineExport(exports, key, () => sourceModule[key])
       }
       catch (_err) { }
     }

packages/vite-node/src/utils.ts

@@ -9,6 +9,10 @@ export function slash(str: string) {
   return str.replace(/\\/g, '/')
 }
 
+export function getType(value: unknown): string {
+  return Object.prototype.toString.apply(value).slice(8, -1)
+}
+
 export function mergeSlashes(str: string) {
   return str.replace(/\/\//g, '/')
 }

test/core/package.json

@@ -6,6 +6,7 @@
     "coverage": "vitest run --coverage"
   },
   "devDependencies": {
+    "tinyspy": "^1.0.2",
     "vitest": "workspace:*"
   }
 }

test/core/src/cjs/array-cjs.js

@@ -0,0 +1 @@
+module.exports = [1, '2']

test/core/src/cjs/bare-cjs.js

@@ -0,0 +1,3 @@
+module.exports = { c: 'c' }
+exports.a = 'a'
+exports.b = 'b'

test/core/src/cjs/class-cjs.js

@@ -0,0 +1,6 @@
+class Test {
+  variable = 1
+}
+
+module.exports = new Test()
+module.exports.Test = Test

test/core/src/cjs/primitive-cjs.js

@@ -0,0 +1,2 @@
+module.exports = 'string'
+exports.a = 'a'

test/core/src/esm/internal-esm.mjs

@@ -0,0 +1 @@
+export * from 'tinyspy'

test/core/test/module.test.ts

@@ -1,20 +1,63 @@
 import { expect, it } from 'vitest'
-// eslint-disable-next-line @typescript-eslint/ban-ts-comment
-// @ts-expect-error
-import { a, b } from '../src/module-cjs'
+// @ts-expect-error is not typed
+import cjs, { a, b } from '../src/cjs/module-cjs'
+// @ts-expect-error is not typed with imports
+import bareCjs, { a as bareA, b as bareB } from '../src/cjs/bare-cjs'
+// @ts-expect-error is not typed with imports
+import primitiveCjs, { a as primitiveA } from '../src/cjs/primitive-cjs'
+// @ts-expect-error is not typed with imports
+import * as primitiveAll from '../src/cjs/primitive-cjs'
+// @ts-expect-error is not typed with imports
+import * as arrayCjs from '../src/cjs/array-cjs'
+// @ts-expect-error is not typed with imports
+import * as classCjs from '../src/cjs/class-cjs'
+// @ts-expect-error is not typed with imports
+import * as internalEsm from '../src/esm/internal-esm.mjs'
 import c, { d } from '../src/module-esm'
 import * as timeout from '../src/timeout'
 
 it('doesn\'t when extending module', () => {
   expect(() => Object.assign(globalThis, timeout)).not.toThrow()
 })
 
-it('should work when using cjs module', () => {
+it('should work when using module.exports cjs', () => {
+  expect(cjs.a).toBe(1)
+  expect(cjs.b).toBe(2)
   expect(a).toBe(1)
   expect(b).toBe(2)
 })
 
+it('works with bare exports cjs', () => {
+  expect(bareCjs.a).toBe('a')
+  expect(bareCjs.b).toBe('b')
+  expect(bareCjs.c).toBe('c')
+  expect(bareA).toBe('a')
+  expect(bareB).toBe('b')
+})
+
+it('primitive cjs retains its logic', () => {
+  expect(primitiveA).toBeUndefined()
+  expect(primitiveCjs).toBe('string')
+  expect(primitiveAll.default).toBe('string')
+  expect(primitiveAll, 'doesn\'t put chars from "string" on exports').not.toHaveProperty('0')
+})
+
+it('arrays-cjs', () => {
+  expect(arrayCjs.default).toEqual([1, '2'])
+  expect(arrayCjs).not.toHaveProperty('0')
+})
+
+it('class-cjs', () => {
+  expect(classCjs.default).toEqual({ variable: 1, Test: expect.any(Function) })
+  expect(classCjs.default).toBeInstanceOf(classCjs.Test)
+  expect(classCjs).not.toHaveProperty('variable')
+})
+
 it('should work when using esm module', () => {
   expect(c).toBe(1)
   expect(d).toBe(2)
 })
+
+it('exports all from native ESM module', () => {
+  expect(internalEsm).toHaveProperty('restoreAll')
+})

test/core/vitest.config.ts

@@ -57,6 +57,9 @@ export default defineConfig({
     sequence: {
       seed: 101,
     },
+    deps: {
+      external: ['tinyspy'],
+    },
     alias: [
       {
         find: 'test-alias',