From 7e1faf3da6d80516b32afb7a14db7fcea6e52ee8 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 5 Nov 2024 19:03:55 +0900 Subject: [PATCH] fix(vite-node): improve esm check to decide external (#6816) --- packages/vite-node/package.json | 1 + packages/vite-node/src/externalize.ts | 18 ++++++++------- pnpm-lock.yaml | 22 +++++++++---------- .../index.js => dep-cjs/esm-comment.js} | 0 test/core/deps/dep-cjs/esm-string.js | 1 + test/core/deps/dep-cjs/package.json | 8 +++++++ test/core/deps/dep-esm-comment/package.json | 5 ----- test/core/package.json | 2 +- test/core/test/dual-package-hazard.test.ts | 12 ++++++++-- 9 files changed, 41 insertions(+), 28 deletions(-) rename test/core/deps/{dep-esm-comment/index.js => dep-cjs/esm-comment.js} (100%) create mode 100644 test/core/deps/dep-cjs/esm-string.js create mode 100644 test/core/deps/dep-cjs/package.json delete mode 100644 test/core/deps/dep-esm-comment/package.json diff --git a/packages/vite-node/package.json b/packages/vite-node/package.json index 7be5029fb6d9..1188ac72bf59 100644 --- a/packages/vite-node/package.json +++ b/packages/vite-node/package.json @@ -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" }, diff --git a/packages/vite-node/src/externalize.ts b/packages/vite-node/src/externalize.ts index 42d7c0526a53..2e3fd0b7d274 100644 --- a/packages/vite-node/src/externalize.ts +++ b/packages/vite-node/src/externalize.ts @@ -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$/, @@ -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>() diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 62763ea9043d..6c51e07a249f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -816,6 +816,9 @@ importers: debug: specifier: ^4.3.7 version: 4.3.7 + es-module-lexer: + specifier: ^1.5.4 + version: 1.5.4 pathe: specifier: ^1.1.2 version: 1.1.2 @@ -1147,9 +1150,9 @@ importers: '@vitest/runner': specifier: workspace:* version: link:../../packages/runner - '@vitest/test-dep-esm-comment': - specifier: file:./deps/dep-esm-comment - version: file:test/core/deps/dep-esm-comment + '@vitest/test-dep-cjs': + specifier: file:./deps/dep-cjs + version: file:test/core/deps/dep-cjs '@vitest/test-dep1': specifier: file:./deps/dep1 version: file:test/core/deps/dep1 @@ -4035,8 +4038,8 @@ packages: vitest: optional: true - '@vitest/test-dep-esm-comment@file:test/core/deps/dep-esm-comment': - resolution: {directory: test/core/deps/dep-esm-comment, type: directory} + '@vitest/test-dep-cjs@file:test/core/deps/dep-cjs': + resolution: {directory: test/core/deps/dep-cjs, type: directory} '@vitest/test-dep1@file:test/core/deps/dep1': resolution: {directory: test/core/deps/dep1, type: directory} @@ -5436,9 +5439,6 @@ packages: es-get-iterator@1.1.3: resolution: {integrity: sha512-sPZmqHBe6JIiTfN5q2pEi//TwxmAFHwj/XEuYjTuse78i8KxaqMTTzxPoFKuzRpDpTJ+0NAbpfenkmH2rePtuw==} - es-module-lexer@1.3.1: - resolution: {integrity: sha512-JUFAyicQV9mXc3YRxPnDlrfBKpqt6hUYzz9/boprUJHs4e4KVr3XwOF70doO6gwXUor6EWZJAyWAfKki84t20Q==} - es-module-lexer@1.5.4: resolution: {integrity: sha512-MVNK56NiMrOwitFB7cqDwq0CQutbw+0BvLshJSse0MUNU+y1FC3bUS/AQg7oUng+/wKrrki7JfmwtVHkVfPLlw==} @@ -12534,7 +12534,7 @@ snapshots: typescript: 5.6.3 vitest: link:packages/vitest - '@vitest/test-dep-esm-comment@file:test/core/deps/dep-esm-comment': {} + '@vitest/test-dep-cjs@file:test/core/deps/dep-cjs': {} '@vitest/test-dep1@file:test/core/deps/dep1': {} @@ -14161,8 +14161,6 @@ snapshots: isarray: 2.0.5 stop-iteration-iterator: 1.0.0 - es-module-lexer@1.3.1: {} - es-module-lexer@1.5.4: {} es-object-atoms@1.0.0: @@ -17371,7 +17369,7 @@ snapshots: dependencies: '@rollup/pluginutils': 5.0.5(rollup@4.24.4) debug: 4.3.7 - es-module-lexer: 1.3.1 + es-module-lexer: 1.5.4 esbuild: 0.24.0 get-tsconfig: 4.8.1 rollup: 4.24.4 diff --git a/test/core/deps/dep-esm-comment/index.js b/test/core/deps/dep-cjs/esm-comment.js similarity index 100% rename from test/core/deps/dep-esm-comment/index.js rename to test/core/deps/dep-cjs/esm-comment.js diff --git a/test/core/deps/dep-cjs/esm-string.js b/test/core/deps/dep-cjs/esm-string.js new file mode 100644 index 000000000000..79f27451ee6d --- /dev/null +++ b/test/core/deps/dep-cjs/esm-string.js @@ -0,0 +1 @@ +module.exports = { test: ' import.meta' } diff --git a/test/core/deps/dep-cjs/package.json b/test/core/deps/dep-cjs/package.json new file mode 100644 index 000000000000..f424b60c62fb --- /dev/null +++ b/test/core/deps/dep-cjs/package.json @@ -0,0 +1,8 @@ +{ + "name": "@vitest/test-dep-cjs", + "type": "commonjs", + "exports": { + "./esm-comment": "./esm-comment.js", + "./esm-string": "./esm-string.js" + } +} diff --git a/test/core/deps/dep-esm-comment/package.json b/test/core/deps/dep-esm-comment/package.json deleted file mode 100644 index 5aa6a8a793e0..000000000000 --- a/test/core/deps/dep-esm-comment/package.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "name": "@vitest/test-dep-esm-comment", - "type": "commonjs", - "exports": "./index.js" -} diff --git a/test/core/package.json b/test/core/package.json index 4e54d2960819..61d675012944 100644 --- a/test/core/package.json +++ b/test/core/package.json @@ -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:*", diff --git a/test/core/test/dual-package-hazard.test.ts b/test/core/test/dual-package-hazard.test.ts index 54d09c57a128..70601831c78b 100644 --- a/test/core/test/dual-package-hazard.test.ts +++ b/test/core/test/dual-package-hazard.test.ts @@ -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) @@ -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) +})