fix: improve CJS-to-ESM transform for better npm compatibility (#3697)

bartlomieju · claude · web-flow · commit d6b0a169a38e · 2026-04-03T16:29:48.000+02:00
## Summary Fixes several issues in the Fresh Vite plugin's CommonJS-to-ESM Babel transform (`commonjs.ts`) that caused npm packages to break during builds while working fine in dev mode. - **Fix `.mts`/`.cts` extension detection** — `.cts` was incorrectly in the ESM branch (should be `.mts`), causing TypeScript CJS files to skip the transform - **Handle `require.resolve()`** — injects `createRequire(import.meta.url)` polyfill so `require.resolve()` calls work in ESM output (#3619) - **Polyfill `__dirname`/`__filename`** — injects `fileURLToPath`/`dirname` based polyfill when these CJS globals are used - **Use `var` instead of `let` for `_default`** — avoids TDZ errors when Rollup reorders declarations in bundled output (#3653) - **Track `module.exports.X` as named exports** — the MemberExpression visitor now handles both `exports.X` and `module.exports.X` patterns Addresses: #3619, #3653 --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
diff --git a/packages/plugin-vite/src/plugins/patches/commonjs.ts b/packages/plugin-vite/src/plugins/patches/commonjs.ts
@@ -14,14 +14,15 @@ export function cjsPlugin(
   const ALIASED = "aliased";
   const REEXPORT = "re-export";
   const NEEDS_REQUIRE_IMPORT = "needsRequireImport";
+  const NEEDS_DIRNAME_IMPORT = "needsDirnameImport";
   const IS_ESM = "isESM";
 
   return {
     name: "fresh-cjs-esm",
     pre(file) {
       const filename = file.opts.filename;
       if (filename) {
-        if (filename.endsWith(".mjs") || filename.endsWith(".cts")) {
+        if (filename.endsWith(".mjs") || filename.endsWith(".mts")) {
           this.set(IS_ESM, true);
         } else if (filename.endsWith(".cjs") || filename.endsWith(".cts")) {
           this.set(IS_ESM, false);
@@ -112,6 +113,64 @@ export function cjsPlugin(
             );
           }
 
+          const needsDirnameImport = state.get(NEEDS_DIRNAME_IMPORT);
+          if (needsDirnameImport) {
+            // Inject:
+            // ```ts
+            // import { fileURLToPath as __cjs_fileURLToPath } from "node:url";
+            // import { dirname as __cjs_dirname } from "node:path";
+            // const __filename = __cjs_fileURLToPath(import.meta.url);
+            // const __dirname = __cjs_dirname(__filename);
+            // ```
+            const fileURLToPathId = t.identifier("__cjs_fileURLToPath");
+            const dirnameId = t.identifier("__cjs_dirname");
+            const importMetaUrl = t.memberExpression(
+              t.metaProperty(
+                t.identifier("import"),
+                t.identifier("meta"),
+              ),
+              t.identifier("url"),
+            );
+
+            path.unshiftContainer(
+              "body",
+              t.variableDeclaration("var", [
+                t.variableDeclarator(
+                  t.identifier("__dirname"),
+                  t.callExpression(dirnameId, [t.identifier("__filename")]),
+                ),
+              ]),
+            );
+            path.unshiftContainer(
+              "body",
+              t.variableDeclaration("var", [
+                t.variableDeclarator(
+                  t.identifier("__filename"),
+                  t.callExpression(fileURLToPathId, [importMetaUrl]),
+                ),
+              ]),
+            );
+            path.unshiftContainer(
+              "body",
+              t.importDeclaration(
+                [t.importSpecifier(dirnameId, t.identifier("dirname"))],
+                t.stringLiteral("node:path"),
+              ),
+            );
+            path.unshiftContainer(
+              "body",
+              t.importDeclaration(
+                [
+                  t.importSpecifier(
+                    fileURLToPathId,
+                    t.identifier("fileURLToPath"),
+                  ),
+                ],
+                t.stringLiteral("node:url"),
+              ),
+            );
+          }
+
           if (reexport !== null) {
             path.unshiftContainer(
               "body",
@@ -246,9 +305,11 @@ export function cjsPlugin(
           if (exported.size > 0 || exportedNs.size > 0 || hasEsModule) {
             const id = path.scope.generateUidIdentifier("__default");
 
+            // Use `var` instead of `const` to avoid TDZ errors when
+            // Rollup reorders declarations in the bundled output.
             path.pushContainer(
               "body",
-              t.variableDeclaration("let", [
+              t.variableDeclaration("var", [
                 t.variableDeclarator(
                   id,
                 ),
@@ -303,6 +364,8 @@ export function cjsPlugin(
               const mapped = mappedNs[i];
 
               const key = path.scope.generateUid("k");
+              // Only spread namespace properties when the module has no
+              // explicit default export (i.e. "default" not in exports).
               path.pushContainer(
                 "body",
                 t.ifStatement(
@@ -430,6 +493,18 @@ export function cjsPlugin(
           return;
         }
 
+        // Handle require.resolve() by injecting createRequire
+        if (
+          t.isMemberExpression(path.node.callee) &&
+          t.isIdentifier(path.node.callee.object) &&
+          path.node.callee.object.name === "require" &&
+          t.isIdentifier(path.node.callee.property) &&
+          path.node.callee.property.name === "resolve"
+        ) {
+          state.set(NEEDS_REQUIRE_IMPORT, true);
+          return;
+        }
+
         if (
           t.isIdentifier(path.node.callee) &&
           path.node.callee.name === "require"
@@ -557,15 +632,21 @@ export function cjsPlugin(
         exit(path, state) {
           if (state.get(IS_ESM)) return;
           if (
-            t.isIdentifier(path.node.object) &&
-            path.node.object.name === "exports" &&
-            t.isIdentifier(path.node.property)
+            t.isIdentifier(path.node.property) &&
+            path.node.property.name !== "__esModule"
           ) {
-            const name = t.cloneNode(path.node.property);
-
-            if (name.name === "__esModule") return;
-
-            state.get(EXPORTED).add(name.name);
+            // Track both `exports.X` and `module.exports.X`
+            if (
+              t.isIdentifier(path.node.object) &&
+              path.node.object.name === "exports"
+            ) {
+              state.get(EXPORTED).add(path.node.property.name);
+            } else if (
+              t.isMemberExpression(path.node.object) &&
+              isModuleExports(t, path.node.object)
+            ) {
+              state.get(EXPORTED).add(path.node.property.name);
+            }
           }
         },
       },
@@ -780,6 +861,20 @@ export function cjsPlugin(
           path.replaceWith(t.cloneNode(path.node.alternate, true));
         }
       },
+      Identifier(path, state) {
+        if (state.get(IS_ESM)) return;
+
+        const name = path.node.name;
+        if (name !== "__dirname" && name !== "__filename") return;
+
+        // Skip if this is already a declaration (e.g. our own polyfill)
+        if (
+          path.parentPath?.isVariableDeclarator() &&
+          path.parentPath.get("id") === path
+        ) return;
+
+        state.set(NEEDS_DIRNAME_IMPORT, true);
+      },
       AssignmentExpression(path, state) {
         if (state.get(IS_ESM)) return;
 
diff --git a/packages/plugin-vite/src/plugins/patches/commonjs_test.ts b/packages/plugin-vite/src/plugins/patches/commonjs_test.ts
@@ -29,7 +29,7 @@ Object.defineProperty(exports, "__esModule", {
   value: true
 });`;
 
-const DEFAULT_EXPORT = `let _default;
+const DEFAULT_EXPORT = `var _default;
 if (typeof exports === "object" && exports !== null && "default" in exports) {
   _default = exports.default;
 } else {
@@ -715,6 +715,92 @@ const node_events_1 = __importDefault({
   });
 });
 
+// --- New tests for CJS transform fixes ---
+
+Deno.test("commonjs - require.resolve injects createRequire", () => {
+  runTest({
+    input: `var resolved = require.resolve("some-package");`,
+    expected: `${IMPORT_REQUIRE}
+var resolved = require.resolve("some-package");`,
+  });
+});
+
+Deno.test("commonjs - require.resolve with require() both inject createRequire once", () => {
+  runTest({
+    input: `var resolved = require.resolve("some-package");
+if (true) {
+  var mod = require("other");
+}`,
+    expected: `${IMPORT_REQUIRE}
+var resolved = require.resolve("some-package");
+if (true) {
+  var mod = require("other");
+}`,
+  });
+});
+
+Deno.test("commonjs - .mts file treated as ESM", () => {
+  runTest({
+    filename: "foo.mts",
+    input: `export const x = 1;`,
+    expected: `export const x = 1;`,
+  });
+});
+
+Deno.test("commonjs - .cts file treated as CJS", () => {
+  runTest({
+    filename: "foo.cts",
+    input: `module.exports = 42;`,
+    expected: `${INIT}
+module.exports = 42;
+${DEFAULT_EXPORT}
+${DEFAULT_EXPORT_END}`,
+  });
+});
+
+Deno.test("commonjs - __dirname and __filename polyfill", () => {
+  const DIRNAME_IMPORT =
+    `import { fileURLToPath as __cjs_fileURLToPath } from "node:url";
+import { dirname as __cjs_dirname } from "node:path";
+var __filename = __cjs_fileURLToPath(import.meta.url);
+var __dirname = __cjs_dirname(__filename);`;
+  runTest({
+    input: `var dir = __dirname;
+var file = __filename;
+module.exports = { dir: dir, file: file };`,
+    expected: `${INIT}
+${DIRNAME_IMPORT}
+var dir = __dirname;
+var file = __filename;
+module.exports = {
+  dir: dir,
+  file: file
+};
+var _dir = exports.dir;
+var _file = exports.file;
+export { _dir as dir, _file as file };
+${DEFAULT_EXPORT}
+${DEFAULT_EXPORT_END}`,
+  });
+});
+
+Deno.test("commonjs - module.exports.X tracked as named export", () => {
+  runTest({
+    input: `module.exports = function parse() {};
+module.exports.parse = module.exports;
+module.exports.stringify = function stringify() {};`,
+    expected: `${INIT}
+module.exports = function parse() {};
+module.exports.parse = module.exports;
+module.exports.stringify = function stringify() {};
+var _parse = exports.parse;
+var _stringify = exports.stringify;
+export { _parse as parse, _stringify as stringify };
+${DEFAULT_EXPORT}
+${DEFAULT_EXPORT_END}`,
+  });
+});
+
 Deno.test("commonjs imitating esm - default export exists", () => {
   runTest({
     input: `module.exports = {
@@ -733,3 +819,17 @@ ${DEFAULT_EXPORT}
 ${DEFAULT_EXPORT_END}`,
   });
 });
+
+Deno.test("commonjs - primitive module.exports with namespace re-export guards assignment", () => {
+  runTest({
+    input: `__exportStar(require("./utils"), exports);
+module.exports = "RFC3986";`,
+    expected: `${INIT}
+import * as _ns from "./utils";
+export * from "./utils";
+module.exports = "RFC3986";
+${DEFAULT_EXPORT}
+if (typeof exports === "object" && exports !== null && !("default" in exports)) for (var _k in _ns) if (_k !== "default" && _k !== "__esModule" && Object.prototype.hasOwnProperty.call(_ns, _k)) _default[_k] = _ns[_k];
+${DEFAULT_EXPORT_END}`,
+  });
+});
