From 498f5a99639c40682f1e8f508d2e5ccb0d53a4f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 27 Apr 2026 22:40:23 +0200 Subject: [PATCH 1/2] fix: CJS detection regression for packages with "export" in comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The content-based heuristic (`code.includes("export ")`) was fooled by CJS files containing the word "export" in comments — e.g. `@opentelemetry/api`'s CJS entry has a comment: // TODO: Remove ProxyTracerProvider export in the next major version. This caused the CJS shim to be skipped, leaving `exports` undefined when Vite's ESModulesEvaluator ran the file. Replace the naive string check with two-layer detection: 1. Package.json "type" field (Node.js semantics): .cjs is always CJS, .js is CJS unless the nearest package.json has "type": "module". Results are cached per directory. 2. ESM statement regex fallback: for dual CJS/ESM packages that ship ESM in .js without "type": "module" (e.g. @opentelemetry/api), detect actual ESM syntax (export/import statements) including minified forms like `import{` and `export*from`. Also adds a test that imports from a real CJS npm package (qs) in node_modules, since the existing CJS test fixtures were local ESM files that never exercised the shim path. --- .../plugin-vite/demo/routes/tests/cjs_npm.tsx | 6 + packages/plugin-vite/src/plugins/deno.ts | 115 ++++++++++++------ packages/plugin-vite/tests/dev_server_test.ts | 11 ++ 3 files changed, 94 insertions(+), 38 deletions(-) create mode 100644 packages/plugin-vite/demo/routes/tests/cjs_npm.tsx diff --git a/packages/plugin-vite/demo/routes/tests/cjs_npm.tsx b/packages/plugin-vite/demo/routes/tests/cjs_npm.tsx new file mode 100644 index 00000000000..e8a333346ba --- /dev/null +++ b/packages/plugin-vite/demo/routes/tests/cjs_npm.tsx @@ -0,0 +1,6 @@ +import qs from "qs"; + +export default function Page() { + const parsed = qs.parse("a=1&b=2"); + return

{parsed.a === "1" ? "qs-ok" : "qs-fail"}

; +} diff --git a/packages/plugin-vite/src/plugins/deno.ts b/packages/plugin-vite/src/plugins/deno.ts index 016529bb593..a2643a001b3 100644 --- a/packages/plugin-vite/src/plugins/deno.ts +++ b/packages/plugin-vite/src/plugins/deno.ts @@ -23,6 +23,37 @@ export function deno(): Plugin { let isDev = false; + // Cache for package.json "type" field lookups. Per Node.js semantics, + // .js files in node_modules are CJS unless the nearest package.json + // has "type": "module". + const pkgTypeCache = new Map(); + async function isEsmPackage(filePath: string): Promise { + let dir = path.dirname(filePath); + while (true) { + const cached = pkgTypeCache.get(dir); + if (cached !== undefined) return cached; + + try { + const text = await Deno.readTextFile(path.join(dir, "package.json")); + const isEsm = JSON.parse(text).type === "module"; + pkgTypeCache.set(dir, isEsm); + return isEsm; + } catch { + const parent = path.dirname(dir); + if (parent === dir) break; + dir = parent; + } + } + return false; + } + + // Detect actual ESM export/import statements at the statement level. + // More robust than code.includes("export ") which matches comments + // and strings (e.g. "// Remove export in next version" would trick it). + // Handles both formatted and minified ESM (e.g. "import{" with no space). + const ESM_STMT_RE = + /(?:^|[\n;])\s*(?:export\s*[{*]|export\s+(?:default|const|let|var|function|class|async)\b|import\s*[{*"']|import\s+[a-zA-Z_$])/; + return { name: "deno", sharedDuringBuild: true, @@ -186,27 +217,34 @@ export function deno(): Plugin { // - SSR: module runner evaluates as ESM, needs module/exports/require // - Client: browser evaluates as ESM, needs module/exports // In build mode, Rollup's @rollup/plugin-commonjs handles CJS. + // + // CJS detection uses Node.js semantics (package.json "type" field) + // instead of content heuristics, which can be fooled by comments + // or strings containing "export"/"import". if ( isDev && !id.startsWith("\0") && id.includes("node_modules") && /\.(c?js|cjs)$/.test(id) ) { - try { - const code = await Deno.readTextFile(id); - // Quick heuristic: if file has CJS patterns and no ESM - if ( - !code.includes("export ") && - !code.includes("import ") && - (code.includes("module.exports") || - code.includes("exports.") || - code.includes("require(")) - ) { - const isServer = this.environment.config.consumer === "server"; - - if (isServer) { - // SSR: use Node.js createRequire for full CJS compat - const wrapped = ` + // .cjs is always CJS. For .js files, check the nearest + // package.json "type" field first (Node.js semantics), then + // fall back to content-based detection for dual CJS/ESM + // packages that ship ESM in .js without "type": "module". + if (id.endsWith(".cjs") || !(await isEsmPackage(id))) { + try { + const code = await Deno.readTextFile(id); + + // Skip if the file contains actual ESM syntax. Some packages + // (e.g. @opentelemetry/api) ship both CJS and ESM as .js + // without "type": "module" in package.json. + if (!ESM_STMT_RE.test(code)) { + const isServer = + this.environment.config.consumer === "server"; + + if (isServer) { + // SSR: use Node.js createRequire for full CJS compat + const wrapped = ` import { createRequire as __cjs_createRequire } from "node:module"; import { fileURLToPath as __cjs_fileURLToPath } from "node:url"; import { dirname as __cjs_dirname } from "node:path"; @@ -220,26 +258,26 @@ ${code} export default module.exports; `; - return { code: wrapped }; - } - - // Client: convert require() calls to ESM imports so - // browsers can load them. Hoist static require() calls - // to import statements at the top. - const imports: string[] = []; - let idx = 0; - const transformed = code.replace( - /\brequire\(["']([^"']+)["']\)/g, - (_match: string, spec: string) => { - const varName = `__cjs_import_${idx++}`; - imports.push( - `import ${varName} from ${JSON.stringify(spec)};`, - ); - return `(${varName}.default ?? ${varName})`; - }, - ); - - const wrapped = `${imports.join("\n")} + return { code: wrapped }; + } + + // Client: convert require() calls to ESM imports so + // browsers can load them. Hoist static require() calls + // to import statements at the top. + const imports: string[] = []; + let idx = 0; + const transformed = code.replace( + /\brequire\(["']([^"']+)["']\)/g, + (_match: string, spec: string) => { + const varName = `__cjs_import_${idx++}`; + imports.push( + `import ${varName} from ${JSON.stringify(spec)};`, + ); + return `(${varName}.default ?? ${varName})`; + }, + ); + + const wrapped = `${imports.join("\n")} var module = { exports: {} }; var exports = module.exports; var __filename = ""; @@ -249,10 +287,11 @@ ${transformed} export default module.exports; `; - return { code: wrapped }; + return { code: wrapped }; + } + } catch { + // Fall through to default loading } - } catch { - // Fall through to default loading } } diff --git a/packages/plugin-vite/tests/dev_server_test.ts b/packages/plugin-vite/tests/dev_server_test.ts index ffc30f92ee2..3eb1d8f6bd9 100644 --- a/packages/plugin-vite/tests/dev_server_test.ts +++ b/packages/plugin-vite/tests/dev_server_test.ts @@ -452,6 +452,17 @@ Deno.test({ sanitizeResources: false, }); +Deno.test({ + name: "vite dev - CJS npm package (qs)", + fn: async () => { + const res = await fetch(`${demoServer.address()}/tests/cjs_npm`); + const text = await res.text(); + expect(text).toContain("

qs-ok

"); + }, + sanitizeOps: false, + sanitizeResources: false, +}); + // issue: https://github.com/denoland/fresh/issues/3666 integrationTest( "vite dev - basePath does not intercept Vite URLs", From 8103820cb6a186178eb59c65612c9e9ad1c6d173 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 28 Apr 2026 08:55:13 +0200 Subject: [PATCH 2/2] fix: deno fmt --- packages/plugin-vite/src/plugins/deno.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/plugin-vite/src/plugins/deno.ts b/packages/plugin-vite/src/plugins/deno.ts index a2643a001b3..2ab1e22ac95 100644 --- a/packages/plugin-vite/src/plugins/deno.ts +++ b/packages/plugin-vite/src/plugins/deno.ts @@ -239,8 +239,7 @@ export function deno(): Plugin { // (e.g. @opentelemetry/api) ship both CJS and ESM as .js // without "type": "module" in package.json. if (!ESM_STMT_RE.test(code)) { - const isServer = - this.environment.config.consumer === "server"; + const isServer = this.environment.config.consumer === "server"; if (isServer) { // SSR: use Node.js createRequire for full CJS compat