Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: improve interchangeability with cjs #1944

Merged
merged 5 commits into from Sep 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
61 changes: 51 additions & 10 deletions packages/vite-node/src/client.ts
Expand Up @@ -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')
Expand Down Expand Up @@ -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
},
}

Expand Down Expand Up @@ -282,7 +308,7 @@ export class ViteNodeRunner {

// cjs compact
require: createRequire(url),
exports,
exports: cjsExports,
module: moduleProxy,
__filename,
__dirname: dirname(__filename),
Expand Down Expand Up @@ -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) { }
}
Expand Down
4 changes: 4 additions & 0 deletions packages/vite-node/src/utils.ts
Expand Up @@ -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, '/')
}
Expand Down
3 changes: 2 additions & 1 deletion pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions test/core/package.json
Expand Up @@ -6,6 +6,7 @@
"coverage": "vitest run --coverage"
},
"devDependencies": {
"tinyspy": "^1.0.2",
"vitest": "workspace:*"
}
}
1 change: 1 addition & 0 deletions test/core/src/cjs/array-cjs.js
@@ -0,0 +1 @@
module.exports = [1, '2']
3 changes: 3 additions & 0 deletions test/core/src/cjs/bare-cjs.js
@@ -0,0 +1,3 @@
module.exports = { c: 'c' }
exports.a = 'a'
exports.b = 'b'
6 changes: 6 additions & 0 deletions test/core/src/cjs/class-cjs.js
@@ -0,0 +1,6 @@
class Test {
variable = 1
}

module.exports = new Test()
module.exports.Test = Test
File renamed without changes.
2 changes: 2 additions & 0 deletions test/core/src/cjs/primitive-cjs.js
@@ -0,0 +1,2 @@
module.exports = 'string'
exports.a = 'a'
1 change: 1 addition & 0 deletions test/core/src/esm/internal-esm.mjs
@@ -0,0 +1 @@
export * from 'tinyspy'
51 changes: 47 additions & 4 deletions 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')
})
3 changes: 3 additions & 0 deletions test/core/vitest.config.ts
Expand Up @@ -57,6 +57,9 @@ export default defineConfig({
sequence: {
seed: 101,
},
deps: {
external: ['tinyspy'],
},
alias: [
{
find: 'test-alias',
Expand Down