Skip to content

Commit 76b9142

Browse files
committed
Revert "fix: CJS detection regression for packages with "export" in comments (#3793)"
This reverts commit 838f41a.
1 parent 2cf68af commit 76b9142

3 files changed

Lines changed: 38 additions & 93 deletions

File tree

packages/plugin-vite/demo/routes/tests/cjs_npm.tsx

Lines changed: 0 additions & 6 deletions
This file was deleted.

packages/plugin-vite/src/plugins/deno.ts

Lines changed: 38 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -23,37 +23,6 @@ export function deno(): Plugin {
2323

2424
let isDev = false;
2525

26-
// Cache for package.json "type" field lookups. Per Node.js semantics,
27-
// .js files in node_modules are CJS unless the nearest package.json
28-
// has "type": "module".
29-
const pkgTypeCache = new Map<string, boolean>();
30-
async function isEsmPackage(filePath: string): Promise<boolean> {
31-
let dir = path.dirname(filePath);
32-
while (true) {
33-
const cached = pkgTypeCache.get(dir);
34-
if (cached !== undefined) return cached;
35-
36-
try {
37-
const text = await Deno.readTextFile(path.join(dir, "package.json"));
38-
const isEsm = JSON.parse(text).type === "module";
39-
pkgTypeCache.set(dir, isEsm);
40-
return isEsm;
41-
} catch {
42-
const parent = path.dirname(dir);
43-
if (parent === dir) break;
44-
dir = parent;
45-
}
46-
}
47-
return false;
48-
}
49-
50-
// Detect actual ESM export/import statements at the statement level.
51-
// More robust than code.includes("export ") which matches comments
52-
// and strings (e.g. "// Remove export in next version" would trick it).
53-
// Handles both formatted and minified ESM (e.g. "import{" with no space).
54-
const ESM_STMT_RE =
55-
/(?:^|[\n;])\s*(?:export\s*[{*]|export\s+(?:default|const|let|var|function|class|async)\b|import\s*[{*"']|import\s+[a-zA-Z_$])/;
56-
5726
return {
5827
name: "deno",
5928
sharedDuringBuild: true,
@@ -217,33 +186,27 @@ export function deno(): Plugin {
217186
// - SSR: module runner evaluates as ESM, needs module/exports/require
218187
// - Client: browser evaluates as ESM, needs module/exports
219188
// In build mode, Rollup's @rollup/plugin-commonjs handles CJS.
220-
//
221-
// CJS detection uses Node.js semantics (package.json "type" field)
222-
// instead of content heuristics, which can be fooled by comments
223-
// or strings containing "export"/"import".
224189
if (
225190
isDev &&
226191
!id.startsWith("\0") &&
227192
id.includes("node_modules") &&
228193
/\.(c?js|cjs)$/.test(id)
229194
) {
230-
// .cjs is always CJS. For .js files, check the nearest
231-
// package.json "type" field first (Node.js semantics), then
232-
// fall back to content-based detection for dual CJS/ESM
233-
// packages that ship ESM in .js without "type": "module".
234-
if (id.endsWith(".cjs") || !(await isEsmPackage(id))) {
235-
try {
236-
const code = await Deno.readTextFile(id);
237-
238-
// Skip if the file contains actual ESM syntax. Some packages
239-
// (e.g. @opentelemetry/api) ship both CJS and ESM as .js
240-
// without "type": "module" in package.json.
241-
if (!ESM_STMT_RE.test(code)) {
242-
const isServer = this.environment.config.consumer === "server";
243-
244-
if (isServer) {
245-
// SSR: use Node.js createRequire for full CJS compat
246-
const wrapped = `
195+
try {
196+
const code = await Deno.readTextFile(id);
197+
// Quick heuristic: if file has CJS patterns and no ESM
198+
if (
199+
!code.includes("export ") &&
200+
!code.includes("import ") &&
201+
(code.includes("module.exports") ||
202+
code.includes("exports.") ||
203+
code.includes("require("))
204+
) {
205+
const isServer = this.environment.config.consumer === "server";
206+
207+
if (isServer) {
208+
// SSR: use Node.js createRequire for full CJS compat
209+
const wrapped = `
247210
import { createRequire as __cjs_createRequire } from "node:module";
248211
import { fileURLToPath as __cjs_fileURLToPath } from "node:url";
249212
import { dirname as __cjs_dirname } from "node:path";
@@ -257,26 +220,26 @@ ${code}
257220
258221
export default module.exports;
259222
`;
260-
return { code: wrapped };
261-
}
262-
263-
// Client: convert require() calls to ESM imports so
264-
// browsers can load them. Hoist static require() calls
265-
// to import statements at the top.
266-
const imports: string[] = [];
267-
let idx = 0;
268-
const transformed = code.replace(
269-
/\brequire\(["']([^"']+)["']\)/g,
270-
(_match: string, spec: string) => {
271-
const varName = `__cjs_import_${idx++}`;
272-
imports.push(
273-
`import ${varName} from ${JSON.stringify(spec)};`,
274-
);
275-
return `(${varName}.default ?? ${varName})`;
276-
},
277-
);
278-
279-
const wrapped = `${imports.join("\n")}
223+
return { code: wrapped };
224+
}
225+
226+
// Client: convert require() calls to ESM imports so
227+
// browsers can load them. Hoist static require() calls
228+
// to import statements at the top.
229+
const imports: string[] = [];
230+
let idx = 0;
231+
const transformed = code.replace(
232+
/\brequire\(["']([^"']+)["']\)/g,
233+
(_match: string, spec: string) => {
234+
const varName = `__cjs_import_${idx++}`;
235+
imports.push(
236+
`import ${varName} from ${JSON.stringify(spec)};`,
237+
);
238+
return `(${varName}.default ?? ${varName})`;
239+
},
240+
);
241+
242+
const wrapped = `${imports.join("\n")}
280243
var module = { exports: {} };
281244
var exports = module.exports;
282245
var __filename = "";
@@ -286,11 +249,10 @@ ${transformed}
286249
287250
export default module.exports;
288251
`;
289-
return { code: wrapped };
290-
}
291-
} catch {
292-
// Fall through to default loading
252+
return { code: wrapped };
293253
}
254+
} catch {
255+
// Fall through to default loading
294256
}
295257
}
296258

packages/plugin-vite/tests/dev_server_test.ts

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -452,17 +452,6 @@ Deno.test({
452452
sanitizeResources: false,
453453
});
454454

455-
Deno.test({
456-
name: "vite dev - CJS npm package (qs)",
457-
fn: async () => {
458-
const res = await fetch(`${demoServer.address()}/tests/cjs_npm`);
459-
const text = await res.text();
460-
expect(text).toContain("<h1>qs-ok</h1>");
461-
},
462-
sanitizeOps: false,
463-
sanitizeResources: false,
464-
});
465-
466455
// issue: https://github.com/denoland/fresh/issues/3666
467456
integrationTest(
468457
"vite dev - basePath does not intercept Vite URLs",

0 commit comments

Comments
 (0)