fix(vite-node): improve esm check to decide external (#6816)

Author: hi-ogawa

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"
   },

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<string, Promise<string | false>>()

pnpm-lock.yaml

@@ -0,0 +1 @@
+module.exports = { test: ' import.meta' }

test/core/deps/dep-esm-comment/index.js test/core/deps/dep-cjs/esm-comment.jstest/core/deps/dep-esm-comment/index.js renamed to test/core/deps/dep-cjs/esm-comment.js

@@ -0,0 +1,8 @@
+{
+  "name": "@vitest/test-dep-cjs",
+  "type": "commonjs",
+  "exports": {
+    "./esm-comment": "./esm-comment.js",
+    "./esm-string": "./esm-string.js"
+  }
+}

test/core/deps/dep-cjs/esm-string.js

@@ -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:*",

test/core/deps/dep-cjs/package.json

@@ -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)
+})