Skip to content

Commit

Permalink
fix(vite-node): improve esm check to decide external (#6816)
Browse files Browse the repository at this point in the history
  • Loading branch information
hi-ogawa authored Nov 5, 2024
1 parent a2175c7 commit 7e1faf3
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 28 deletions.
1 change: 1 addition & 0 deletions packages/vite-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
"dependencies": {
"cac": "^6.7.14",
"debug": "^4.3.7",
"es-module-lexer": "^1.5.4",
"pathe": "^1.1.2",
"vite": "^5.0.0"
},
Expand Down
18 changes: 10 additions & 8 deletions packages/vite-node/src/externalize.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,15 @@
import type { DepsHandlingOptions } from './types'
import { existsSync, promises as fsp } from 'node:fs'
import * as esModuleLexer from 'es-module-lexer'
import { dirname, extname, join } from 'pathe'
import { KNOWN_ASSET_RE } from './constants'
import { findNearestPackageData, isNodeBuiltin, slash } from './utils'

const BUILTIN_EXTENSIONS = new Set(['.mjs', '.cjs', '.node', '.wasm'])

const ESM_SYNTAX_RE
= /(?:[\s;]|^)(?:import[\s\w*,{}]*from|import\s*["'*{]|export\b\s*(?:[*{]|default|class|type|function|const|var|let|async function)|import\.meta\b)/m
const ESM_EXT_RE = /\.(es|esm|esm-browser|esm-bundler|es6|module)\.js$/
const ESM_FOLDER_RE = /\/(es|esm)\/(.*\.js)$/

// https://stackoverflow.com/a/15123777
const COMMENT_RE = /\/\*[\s\S]*?\*\/|([^\\:]|^)\/\/.*$/gm

const defaultInline = [
/virtual:/,
/\.[mc]?ts$/,
Expand Down Expand Up @@ -80,9 +76,15 @@ async function isValidNodeImport(id: string) {
return false
}

const code = await fsp.readFile(id, 'utf8').catch(() => '')

return !ESM_SYNTAX_RE.test(code.replace(COMMENT_RE, ''))
try {
await esModuleLexer.init
const code = await fsp.readFile(id, 'utf8')
const [, , , hasModuleSyntax] = esModuleLexer.parse(code)
return !hasModuleSyntax
}
catch {
return false
}
}

const _defaultExternalizeCache = new Map<string, Promise<string | false>>()
Expand Down
22 changes: 10 additions & 12 deletions pnpm-lock.yaml

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

File renamed without changes.
1 change: 1 addition & 0 deletions test/core/deps/dep-cjs/esm-string.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = { test: ' import.meta' }
8 changes: 8 additions & 0 deletions test/core/deps/dep-cjs/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@vitest/test-dep-cjs",
"type": "commonjs",
"exports": {
"./esm-comment": "./esm-comment.js",
"./esm-string": "./esm-string.js"
}
}
5 changes: 0 additions & 5 deletions test/core/deps/dep-esm-comment/package.json

This file was deleted.

2 changes: 1 addition & 1 deletion test/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"@vitest/expect": "workspace:*",
"@vitest/mocker": "workspace:*",
"@vitest/runner": "workspace:*",
"@vitest/test-dep-esm-comment": "file:./deps/dep-esm-comment",
"@vitest/test-dep-cjs": "file:./deps/dep-cjs",
"@vitest/test-dep1": "file:./deps/dep1",
"@vitest/test-dep2": "file:./deps/dep2",
"@vitest/utils": "workspace:*",
Expand Down
12 changes: 10 additions & 2 deletions test/core/test/dual-package-hazard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import * as dep1 from '@vitest/test-dep1'
import * as dep2 from '@vitest/test-dep2'

// @ts-expect-error no ts
import depEsmComment from '@vitest/test-dep-esm-comment'
import depEsmComment from '@vitest/test-dep-cjs/esm-comment'

// @ts-expect-error no ts
import depEsmString from '@vitest/test-dep-cjs/esm-string'

const require = createRequire(import.meta.url)

Expand All @@ -18,6 +21,11 @@ test('no dual package hazard by externalizing esm deps by default', async () =>
})

test('externalize cjs with esm comment', async () => {
const depEsmCommentRequire = require('@vitest/test-dep-esm-comment')
const depEsmCommentRequire = require('@vitest/test-dep-cjs/esm-comment')
expect(depEsmComment).toBe(depEsmCommentRequire)
})

test('externalize cjs with esm string', async () => {
const depEsmStringRequire = require('@vitest/test-dep-cjs/esm-string')
expect(depEsmString).toBe(depEsmStringRequire)
})

0 comments on commit 7e1faf3

Please sign in to comment.