From 58e5adbcc497373b4af812412336980baad8dff5 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Wed, 31 Aug 2022 14:54:16 +0300 Subject: [PATCH 1/5] fix: allow accessing named cjs exports on default --- packages/vite-node/src/client.ts | 19 +++++++++++++++++-- test/core/test/module.test.ts | 4 +++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/vite-node/src/client.ts b/packages/vite-node/src/client.ts index 2425c34e9791..98ec3340c5a8 100644 --- a/packages/vite-node/src/client.ts +++ b/packages/vite-node/src/client.ts @@ -235,12 +235,27 @@ export class ViteNodeRunner { // disambiguate the `:/` on windows: see nodejs/node#31710 const url = pathToFileURL(fsPath).href const meta = { url } - const exports: any = Object.create(null) - Object.defineProperty(exports, Symbol.toStringTag, { + const exportsBase: any = Object.create(null) + Object.defineProperty(exportsBase, Symbol.toStringTag, { value: 'Module', enumerable: false, configurable: false, }) + const exports = new Proxy(exportsBase, { + set(target, p, value, receiver) { + const result = Reflect.set(target, p, value, receiver) + if (p !== 'default') { + if (!Reflect.has(target, 'default')) + target.default = {} + + // Node also allows access of named exports via exports.default + // https://nodejs.org/api/esm.html#commonjs-namespaces + if (target.default && typeof target.default === 'object') + target.default[p] = value + } + return result + }, + }) Object.assign(mod, { code: transformed, exports }) diff --git a/test/core/test/module.test.ts b/test/core/test/module.test.ts index e34875140fa6..c040b876ce3a 100644 --- a/test/core/test/module.test.ts +++ b/test/core/test/module.test.ts @@ -1,7 +1,7 @@ import { expect, it } from 'vitest' // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-expect-error -import { a, b } from '../src/module-cjs' +import cjs, { a, b } from '../src/module-cjs' import c, { d } from '../src/module-esm' import * as timeout from '../src/timeout' @@ -10,6 +10,8 @@ it('doesn\'t when extending module', () => { }) it('should work when using cjs module', () => { + expect(cjs.a).toBe(1) + expect(cjs.b).toBe(2) expect(a).toBe(1) expect(b).toBe(2) }) From 8a8490722d53fc6b903ffc08c6e6e5c1d6bc084f Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Sat, 3 Sep 2022 11:06:31 +0300 Subject: [PATCH 2/5] feat: improve interchangeability for cjs --- packages/vite-node/src/client.ts | 63 +++++++++++++++++---------- test/core/src/cjs/bare-cjs.js | 3 ++ test/core/src/{ => cjs}/module-cjs.ts | 0 test/core/src/cjs/primitive-cjs.js | 2 + test/core/test/module.test.ts | 28 ++++++++++-- 5 files changed, 70 insertions(+), 26 deletions(-) create mode 100644 test/core/src/cjs/bare-cjs.js rename test/core/src/{ => cjs}/module-cjs.ts (100%) create mode 100644 test/core/src/cjs/primitive-cjs.js diff --git a/packages/vite-node/src/client.ts b/packages/vite-node/src/client.ts index 98ec3340c5a8..9a352fcb912b 100644 --- a/packages/vite-node/src/client.ts +++ b/packages/vite-node/src/client.ts @@ -235,25 +235,36 @@ export class ViteNodeRunner { // disambiguate the `:/` on windows: see nodejs/node#31710 const url = pathToFileURL(fsPath).href const meta = { url } - const exportsBase: any = Object.create(null) - Object.defineProperty(exportsBase, Symbol.toStringTag, { + const exports: any = Object.create(null) + Object.defineProperty(exports, Symbol.toStringTag, { value: 'Module', enumerable: false, configurable: false, }) - const exports = new Proxy(exportsBase, { - set(target, p, value, receiver) { - const result = Reflect.set(target, p, value, receiver) + // 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(target, 'default')) - target.default = {} - - // Node also allows access of named exports via exports.default - // https://nodejs.org/api/esm.html#commonjs-namespaces - if (target.default && typeof target.default === 'object') - target.default[p] = value + 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 result + return Reflect.set(exports, p, value) }, }) @@ -262,11 +273,11 @@ export class ViteNodeRunner { 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 }, } @@ -297,7 +308,7 @@ export class ViteNodeRunner { // cjs compact require: createRequire(url), - exports, + exports: cjsExports, module: moduleProxy, __filename, __dirname: dirname(__filename), @@ -378,20 +389,28 @@ 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 + if (sourceModule === null || typeof sourceModule !== '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) { } } diff --git a/test/core/src/cjs/bare-cjs.js b/test/core/src/cjs/bare-cjs.js new file mode 100644 index 000000000000..3f42b8876380 --- /dev/null +++ b/test/core/src/cjs/bare-cjs.js @@ -0,0 +1,3 @@ +module.exports = { c: 'c' } +exports.a = 'a' +exports.b = 'b' diff --git a/test/core/src/module-cjs.ts b/test/core/src/cjs/module-cjs.ts similarity index 100% rename from test/core/src/module-cjs.ts rename to test/core/src/cjs/module-cjs.ts diff --git a/test/core/src/cjs/primitive-cjs.js b/test/core/src/cjs/primitive-cjs.js new file mode 100644 index 000000000000..a4e52bd6a831 --- /dev/null +++ b/test/core/src/cjs/primitive-cjs.js @@ -0,0 +1,2 @@ +module.exports = 'string' +exports.a = 'a' diff --git a/test/core/test/module.test.ts b/test/core/test/module.test.ts index c040b876ce3a..6965ea078576 100644 --- a/test/core/test/module.test.ts +++ b/test/core/test/module.test.ts @@ -1,7 +1,12 @@ import { expect, it } from 'vitest' -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-expect-error -import cjs, { 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' import c, { d } from '../src/module-esm' import * as timeout from '../src/timeout' @@ -9,13 +14,28 @@ 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('should work when using esm module', () => { expect(c).toBe(1) expect(d).toBe(2) From 891c6ef101a9c748192b200e5fa7ae4c2908fd6c Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Sat, 3 Sep 2022 11:13:36 +0300 Subject: [PATCH 3/5] chore: don't put array elements on exports --- packages/vite-node/src/client.ts | 2 +- test/core/src/cjs/array-cjs.js | 1 + test/core/test/module.test.ts | 7 +++++++ 3 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 test/core/src/cjs/array-cjs.js diff --git a/packages/vite-node/src/client.ts b/packages/vite-node/src/client.ts index 9a352fcb912b..c1ccf40da2f4 100644 --- a/packages/vite-node/src/client.ts +++ b/packages/vite-node/src/client.ts @@ -404,7 +404,7 @@ function exportAll(exports: any, sourceModule: any) { if (exports === sourceModule) return - if (sourceModule === null || typeof sourceModule !== 'object') + if (sourceModule === null || typeof sourceModule !== 'object' || Array.isArray(sourceModule)) return for (const key in sourceModule) { diff --git a/test/core/src/cjs/array-cjs.js b/test/core/src/cjs/array-cjs.js new file mode 100644 index 000000000000..f33365f6143f --- /dev/null +++ b/test/core/src/cjs/array-cjs.js @@ -0,0 +1 @@ +module.exports = [1, '2'] diff --git a/test/core/test/module.test.ts b/test/core/test/module.test.ts index 6965ea078576..288dcb036c6c 100644 --- a/test/core/test/module.test.ts +++ b/test/core/test/module.test.ts @@ -7,6 +7,8 @@ import bareCjs, { a as bareA, b as bareB } from '../src/cjs/bare-cjs' 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' import c, { d } from '../src/module-esm' import * as timeout from '../src/timeout' @@ -36,6 +38,11 @@ it('primitive cjs retains its logic', () => { 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('should work when using esm module', () => { expect(c).toBe(1) expect(d).toBe(2) From e35ecb831b7c368d8a381bd23ef73890dcd98a55 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Sat, 3 Sep 2022 11:24:48 +0300 Subject: [PATCH 4/5] fix: don't put class properties on exports --- packages/vite-node/src/client.ts | 11 +++++++++-- packages/vite-node/src/utils.ts | 4 ++++ test/core/src/cjs/class-cjs.js | 6 ++++++ test/core/test/module.test.ts | 8 ++++++++ 4 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 test/core/src/cjs/class-cjs.js diff --git a/packages/vite-node/src/client.ts b/packages/vite-node/src/client.ts index c1ccf40da2f4..b9cff3267fe9 100644 --- a/packages/vite-node/src/client.ts +++ b/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') @@ -404,7 +404,14 @@ function exportAll(exports: any, sourceModule: any) { if (exports === sourceModule) return - if (sourceModule === null || typeof sourceModule !== 'object' || Array.isArray(sourceModule)) + 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) { diff --git a/packages/vite-node/src/utils.ts b/packages/vite-node/src/utils.ts index b5c2d933d59e..84eb5604d09e 100644 --- a/packages/vite-node/src/utils.ts +++ b/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, '/') } diff --git a/test/core/src/cjs/class-cjs.js b/test/core/src/cjs/class-cjs.js new file mode 100644 index 000000000000..6b7177f1f907 --- /dev/null +++ b/test/core/src/cjs/class-cjs.js @@ -0,0 +1,6 @@ +class Test { + variable = 1 +} + +module.exports = new Test() +module.exports.Test = Test diff --git a/test/core/test/module.test.ts b/test/core/test/module.test.ts index 288dcb036c6c..dac95dda698e 100644 --- a/test/core/test/module.test.ts +++ b/test/core/test/module.test.ts @@ -9,6 +9,8 @@ import primitiveCjs, { a as primitiveA } from '../src/cjs/primitive-cjs' 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' import c, { d } from '../src/module-esm' import * as timeout from '../src/timeout' @@ -43,6 +45,12 @@ it('arrays-cjs', () => { 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) From 2fc377d22abffcd8fb2c77b9365d0240e636d3b9 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Sat, 3 Sep 2022 11:44:15 +0300 Subject: [PATCH 5/5] test: can export all from Native ESM --- pnpm-lock.yaml | 3 ++- test/core/package.json | 1 + test/core/src/esm/internal-esm.mjs | 1 + test/core/test/module.test.ts | 6 ++++++ test/core/vitest.config.ts | 3 +++ 5 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 test/core/src/esm/internal-esm.mjs diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a260ac074a66..529393298200 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -907,8 +907,10 @@ importers: test/core: specifiers: + tinyspy: ^1.0.2 vitest: workspace:* devDependencies: + tinyspy: 1.0.2 vitest: link:../../packages/vitest test/coverage-test: @@ -19197,7 +19199,6 @@ packages: /tinyspy/1.0.2: resolution: {integrity: sha512-bSGlgwLBYf7PnUsQ6WOc6SJ3pGOcd+d8AA6EUnLDDM0kWEstC1JIlSZA3UNliDXhd9ABoS7hiRBDCu+XP/sf1Q==} engines: {node: '>=14.0.0'} - dev: false /tmp/0.0.33: resolution: {integrity: sha512-jRCJlojKnZ3addtTOjdIqoRuPEKBvNXcGYqzO6zWZX8KfKEpnGY5jfggJQ3EjKuu8D4bJRr0y+cYJFmYbImXGw==} diff --git a/test/core/package.json b/test/core/package.json index 04d6e0b02701..a31e9517a9bc 100644 --- a/test/core/package.json +++ b/test/core/package.json @@ -6,6 +6,7 @@ "coverage": "vitest run --coverage" }, "devDependencies": { + "tinyspy": "^1.0.2", "vitest": "workspace:*" } } diff --git a/test/core/src/esm/internal-esm.mjs b/test/core/src/esm/internal-esm.mjs new file mode 100644 index 000000000000..bd693c45a4e2 --- /dev/null +++ b/test/core/src/esm/internal-esm.mjs @@ -0,0 +1 @@ +export * from 'tinyspy' diff --git a/test/core/test/module.test.ts b/test/core/test/module.test.ts index dac95dda698e..73425312da6a 100644 --- a/test/core/test/module.test.ts +++ b/test/core/test/module.test.ts @@ -11,6 +11,8 @@ import * as primitiveAll from '../src/cjs/primitive-cjs' 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' @@ -55,3 +57,7 @@ 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') +}) diff --git a/test/core/vitest.config.ts b/test/core/vitest.config.ts index a0acc89392e5..97d043a26138 100644 --- a/test/core/vitest.config.ts +++ b/test/core/vitest.config.ts @@ -57,6 +57,9 @@ export default defineConfig({ sequence: { seed: 101, }, + deps: { + external: ['tinyspy'], + }, alias: [ { find: 'test-alias',