Skip to content

Commit c31b7c6

Browse files
bartlomiejuclaude
bartlomieju
and
claude
committed
fix(vite-plugin): improve CJS-to-ESM transform for better npm compatibility
Fixes several issues in the CommonJS Babel transform that caused npm packages to break during builds while working in dev mode: - Fix .mts/.cts extension detection (`.cts` was in the ESM branch) - Handle `require.resolve()` by injecting `createRequire` polyfill - Polyfill `__dirname` and `__filename` CJS globals in transformed modules - Use `var` instead of `const` for `_default` to avoid TDZ errors in Rollup bundles - Guard namespace property spreading against primitive default exports - Track `module.exports.X` patterns as named exports (not just `exports.X`) Addresses: #3492, #3619, #3653, #3449, #3505 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4cc76ae commit c31b7c6

2 files changed

Lines changed: 269 additions & 60 deletions

File tree

‎packages/plugin-vite/src/plugins/patches/commonjs.ts‎

Lines changed: 160 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,15 @@ export function cjsPlugin(
1414
const ALIASED = "aliased";
1515
const REEXPORT = "re-export";
1616
const NEEDS_REQUIRE_IMPORT = "needsRequireImport";
17+
const NEEDS_DIRNAME_IMPORT = "needsDirnameImport";
1718
const IS_ESM = "isESM";
1819

1920
return {
2021
name: "fresh-cjs-esm",
2122
pre(file) {
2223
const filename = file.opts.filename;
2324
if (filename) {
24-
if (filename.endsWith(".mjs") || filename.endsWith(".cts")) {
25+
if (filename.endsWith(".mjs") || filename.endsWith(".mts")) {
2526
this.set(IS_ESM, true);
2627
} else if (filename.endsWith(".cjs") || filename.endsWith(".cts")) {
2728
this.set(IS_ESM, false);
@@ -112,6 +113,64 @@ export function cjsPlugin(
112113
);
113114
}
114115

116+
const needsDirnameImport = state.get(NEEDS_DIRNAME_IMPORT);
117+
if (needsDirnameImport) {
118+
// Inject:
119+
// ```ts
120+
// import { fileURLToPath as __cjs_fileURLToPath } from "node:url";
121+
// import { dirname as __cjs_dirname } from "node:path";
122+
// const __filename = __cjs_fileURLToPath(import.meta.url);
123+
// const __dirname = __cjs_dirname(__filename);
124+
// ```
125+
const fileURLToPathId = t.identifier("__cjs_fileURLToPath");
126+
const dirnameId = t.identifier("__cjs_dirname");
127+
const importMetaUrl = t.memberExpression(
128+
t.metaProperty(
129+
t.identifier("import"),
130+
t.identifier("meta"),
131+
),
132+
t.identifier("url"),
133+
);
134+
135+
path.unshiftContainer(
136+
"body",
137+
t.variableDeclaration("var", [
138+
t.variableDeclarator(
139+
t.identifier("__dirname"),
140+
t.callExpression(dirnameId, [t.identifier("__filename")]),
141+
),
142+
]),
143+
);
144+
path.unshiftContainer(
145+
"body",
146+
t.variableDeclaration("var", [
147+
t.variableDeclarator(
148+
t.identifier("__filename"),
149+
t.callExpression(fileURLToPathId, [importMetaUrl]),
150+
),
151+
]),
152+
);
153+
path.unshiftContainer(
154+
"body",
155+
t.importDeclaration(
156+
[t.importSpecifier(dirnameId, t.identifier("dirname"))],
157+
t.stringLiteral("node:path"),
158+
),
159+
);
160+
path.unshiftContainer(
161+
"body",
162+
t.importDeclaration(
163+
[
164+
t.importSpecifier(
165+
fileURLToPathId,
166+
t.identifier("fileURLToPath"),
167+
),
168+
],
169+
t.stringLiteral("node:url"),
170+
),
171+
);
172+
}
173+
115174
if (reexport !== null) {
116175
path.unshiftContainer(
117176
"body",
@@ -251,9 +310,11 @@ export function cjsPlugin(
251310
if (exportNamed.size > 0 || exportedNs.size > 0 || hasEsModule) {
252311
const id = path.scope.generateUidIdentifier("__default");
253312

313+
// Use `var` instead of `const` to avoid TDZ errors when
314+
// Rollup reorders declarations in the bundled output.
254315
path.pushContainer(
255316
"body",
256-
t.variableDeclaration("const", [
317+
t.variableDeclaration("var", [
257318
t.variableDeclarator(
258319
id,
259320
t.logicalExpression(
@@ -272,55 +333,74 @@ export function cjsPlugin(
272333
const mapped = mappedNs[i];
273334

274335
const key = path.scope.generateUid("k");
336+
// Guard the for-in body with a typeof check so that
337+
// namespace properties are not assigned onto a primitive
338+
// default (e.g. when exports.default is a string).
275339
path.pushContainer(
276340
"body",
277-
t.forInStatement(
278-
t.variableDeclaration("var", [
279-
t.variableDeclarator(t.identifier(key)),
280-
]),
281-
t.identifier(mapped),
282-
t.ifStatement(
283-
t.logicalExpression(
284-
"&&",
341+
t.ifStatement(
342+
t.logicalExpression(
343+
"&&",
344+
t.binaryExpression(
345+
"!==",
346+
t.unaryExpression("typeof", t.cloneNode(id, true)),
347+
t.stringLiteral("object"),
348+
),
349+
t.binaryExpression(
350+
"!==",
351+
t.unaryExpression("typeof", t.cloneNode(id, true)),
352+
t.stringLiteral("function"),
353+
),
354+
),
355+
t.emptyStatement(),
356+
t.forInStatement(
357+
t.variableDeclaration("var", [
358+
t.variableDeclarator(t.identifier(key)),
359+
]),
360+
t.identifier(mapped),
361+
t.ifStatement(
285362
t.logicalExpression(
286363
"&&",
287-
t.binaryExpression(
288-
"!==",
289-
t.identifier(key),
290-
t.stringLiteral("default"),
291-
),
292-
t.binaryExpression(
293-
"!==",
294-
t.identifier(key),
295-
t.stringLiteral("__esModule"),
364+
t.logicalExpression(
365+
"&&",
366+
t.binaryExpression(
367+
"!==",
368+
t.identifier(key),
369+
t.stringLiteral("default"),
370+
),
371+
t.binaryExpression(
372+
"!==",
373+
t.identifier(key),
374+
t.stringLiteral("__esModule"),
375+
),
296376
),
297-
),
298-
t.callExpression(
299-
t.memberExpression(
377+
t.callExpression(
300378
t.memberExpression(
301379
t.memberExpression(
302-
t.identifier("Object"),
303-
t.identifier("prototype"),
380+
t.memberExpression(
381+
t.identifier("Object"),
382+
t.identifier("prototype"),
383+
),
384+
t.identifier("hasOwnProperty"),
304385
),
305-
t.identifier("hasOwnProperty"),
386+
t.identifier("call"),
306387
),
307-
t.identifier("call"),
388+
[t.identifier(mapped), t.identifier(key)],
308389
),
309-
[t.identifier(mapped), t.identifier(key)],
310390
),
311-
),
312-
t.expressionStatement(
313-
t.assignmentExpression(
314-
"=",
315-
t.memberExpression(
316-
t.cloneNode(id, true),
317-
t.identifier(key),
318-
true,
319-
),
320-
t.memberExpression(
321-
t.identifier(mapped),
322-
t.identifier(key),
323-
true,
391+
t.expressionStatement(
392+
t.assignmentExpression(
393+
"=",
394+
t.memberExpression(
395+
t.cloneNode(id, true),
396+
t.identifier(key),
397+
true,
398+
),
399+
t.memberExpression(
400+
t.identifier(mapped),
401+
t.identifier(key),
402+
true,
403+
),
324404
),
325405
),
326406
),
@@ -373,6 +453,18 @@ export function cjsPlugin(
373453
return;
374454
}
375455

456+
// Handle require.resolve() by injecting createRequire
457+
if (
458+
t.isMemberExpression(path.node.callee) &&
459+
t.isIdentifier(path.node.callee.object) &&
460+
path.node.callee.object.name === "require" &&
461+
t.isIdentifier(path.node.callee.property) &&
462+
path.node.callee.property.name === "resolve"
463+
) {
464+
state.set(NEEDS_REQUIRE_IMPORT, true);
465+
return;
466+
}
467+
376468
if (
377469
t.isIdentifier(path.node.callee) &&
378470
path.node.callee.name === "require"
@@ -500,15 +592,21 @@ export function cjsPlugin(
500592
exit(path, state) {
501593
if (state.get(IS_ESM)) return;
502594
if (
503-
t.isIdentifier(path.node.object) &&
504-
path.node.object.name === "exports" &&
505-
t.isIdentifier(path.node.property)
595+
t.isIdentifier(path.node.property) &&
596+
path.node.property.name !== "__esModule"
506597
) {
507-
const name = t.cloneNode(path.node.property);
508-
509-
if (name.name === "__esModule") return;
510-
511-
state.get(EXPORTED).add(name.name);
598+
// Track both `exports.X` and `module.exports.X`
599+
if (
600+
t.isIdentifier(path.node.object) &&
601+
path.node.object.name === "exports"
602+
) {
603+
state.get(EXPORTED).add(path.node.property.name);
604+
} else if (
605+
t.isMemberExpression(path.node.object) &&
606+
isModuleExports(t, path.node.object)
607+
) {
608+
state.get(EXPORTED).add(path.node.property.name);
609+
}
512610
}
513611
},
514612
},
@@ -722,6 +820,20 @@ export function cjsPlugin(
722820
path.replaceWith(t.cloneNode(path.node.alternate, true));
723821
}
724822
},
823+
Identifier(path, state) {
824+
if (state.get(IS_ESM)) return;
825+
826+
const name = path.node.name;
827+
if (name !== "__dirname" && name !== "__filename") return;
828+
829+
// Skip if this is already a declaration (e.g. our own polyfill)
830+
if (
831+
path.parentPath?.isVariableDeclarator() &&
832+
path.parentPath.get("id") === path
833+
) return;
834+
835+
state.set(NEEDS_DIRNAME_IMPORT, true);
836+
},
725837
AssignmentExpression(path, state) {
726838
if (state.get(IS_ESM)) return;
727839

0 commit comments

Comments
 (0)