fix: de-optimize route chunks with shared exported declarations

contributors.yml

@@ -97,6 +97,7 @@
 - clonemycode
 - Cmoen11
 - codeape2
+- comp615
 - coolport
 - coryhouse
 - csamuel

integration/split-route-modules-test.ts

@@ -121,7 +121,9 @@ const files = {
     // chunk. The variable name is globally unique to prevent name mangling,
     // e.g. inUnsplittableMainChunk$1. The use of console.log prevents dead code
     // elimination in the build by introducing a side effect
-    export const inUnsplittableMainChunk = () => console.log() || true;
+    export function inUnsplittableMainChunk() {
+      return console.log() || true;
+    }
 
     export const clientLoader = async () => {
       inUnsplittableMainChunk();
@@ -177,7 +179,9 @@ const files = {
     // chunk. The variable name is globally unique to prevent name mangling,
     // e.g. inMixedMainChunk$1. The use of console.log prevents dead code
     // elimination in the build by introducing a side effect
-    export const inMixedMainChunk = () => console.log() || true;
+    export function inMixedMainChunk() {
+      return console.log() || true;
+    }
 
     export const clientLoader = async () => {
       inMixedMainChunk();

packages/react-router-dev/.changes/patch.split-route-export-dependencies.md

@@ -0,0 +1 @@
+Avoid broken route chunks when exported function or class declarations are shared by multiple route exports

packages/react-router-dev/vite/route-chunks-test.ts

@@ -11,17 +11,19 @@ let cache: [Cache, string] = [new Map(), "cacheKey"];
 
 describe("route chunks", () => {
   describe("chunkable", () => {
-    test("functions with no identifiers", () => {
+    test("independent declarations with no identifiers", () => {
       const code = dedent`
         export default function () { return null; }
         export function target1() { return null; }
         export function other1() { return null; }
+        export class OtherClass {}
         export const target2 = () => null;
         export const other2 = () => null;
       `;
       expect(hasChunkableExport(code, "default", ...cache)).toBe(true);
       expect(hasChunkableExport(code, "target1", ...cache)).toBe(true);
       expect(hasChunkableExport(code, "target2", ...cache)).toBe(true);
+      expect(hasChunkableExport(code, "OtherClass", ...cache)).toBe(true);
       expect(getChunkedExport(code, "default", {}, ...cache)?.code)
         .toMatchInlineSnapshot(`
         "export default function () {
@@ -37,10 +39,13 @@ describe("route chunks", () => {
       expect(
         getChunkedExport(code, "target2", {}, ...cache)?.code,
       ).toMatchInlineSnapshot(`"export const target2 = () => null;"`);
+      expect(
+        getChunkedExport(code, "OtherClass", {}, ...cache)?.code,
+      ).toMatchInlineSnapshot(`"export class OtherClass {}"`);
       expect(
         omitChunkedExports(
           code,
-          ["default", "target1", "target2"],
+          ["default", "target1", "target2", "OtherClass"],
           {},
           ...cache,
         )?.code,
@@ -716,6 +721,108 @@ describe("route chunks", () => {
       `);
     });
 
+    test("function depending on an exported function declaration", () => {
+      const code = dedent`
+        export function exportedHelper() {
+          return "loaded";
+        }
+        export function clientLoader() {
+          return exportedHelper();
+        }
+        export default function Component() {
+          return null;
+        }
+      `;
+
+      expect(hasChunkableExport(code, "exportedHelper", ...cache)).toBe(false);
+      expect(hasChunkableExport(code, "clientLoader", ...cache)).toBe(false);
+      expect(hasChunkableExport(code, "default", ...cache)).toBe(true);
+      expect(
+        getChunkedExport(code, "clientLoader", {}, ...cache),
+      ).toBeUndefined();
+      expect(omitChunkedExports(code, ["clientLoader"], {}, ...cache)?.code)
+        .toMatchInlineSnapshot(`
+        "export function exportedHelper() {
+          return "loaded";
+        }
+        export function clientLoader() {
+          return exportedHelper();
+        }
+        export default function Component() {
+          return null;
+        }"
+      `);
+    });
+
+    test("function depending transitively on an exported class declaration", () => {
+      const code = dedent`
+        export class ExportedClass {
+          message = "loaded";
+        }
+        const createExportedClass = () => new ExportedClass();
+        export function clientLoader() {
+          return createExportedClass();
+        }
+        export default function Component() {
+          return null;
+        }
+      `;
+
+      expect(hasChunkableExport(code, "ExportedClass", ...cache)).toBe(false);
+      expect(hasChunkableExport(code, "clientLoader", ...cache)).toBe(false);
+      expect(hasChunkableExport(code, "default", ...cache)).toBe(true);
+      expect(
+        getChunkedExport(code, "clientLoader", {}, ...cache),
+      ).toBeUndefined();
+    });
+
+    test("retained default export depending on an exported route function", () => {
+      const code = dedent`
+        export function clientLoader() {
+          return "loaded";
+        }
+        export default function Component() {
+          return clientLoader();
+        }
+      `;
+
+      expect(hasChunkableExport(code, "clientLoader", ...cache)).toBe(false);
+      expect(hasChunkableExport(code, "default", ...cache)).toBe(false);
+      expect(
+        getChunkedExport(code, "clientLoader", {}, ...cache),
+      ).toBeUndefined();
+      expect(omitChunkedExports(code, ["clientLoader"], {}, ...cache)?.code)
+        .toMatchInlineSnapshot(`
+        "export function clientLoader() {
+          return "loaded";
+        }
+        export default function Component() {
+          return clientLoader();
+        }"
+      `);
+    });
+
+    test.each([
+      ["function", "export default function Component() {}"],
+      ["class", "export default class Component {}"],
+    ])(
+      "export depending on a named default %s declaration",
+      (_, defaultDeclaration) => {
+        const code = dedent`
+          ${defaultDeclaration}
+          export function clientLoader() {
+            return Component.name;
+          }
+        `;
+
+        expect(hasChunkableExport(code, "default", ...cache)).toBe(false);
+        expect(hasChunkableExport(code, "clientLoader", ...cache)).toBe(false);
+        expect(
+          getChunkedExport(code, "clientLoader", {}, ...cache),
+        ).toBeUndefined();
+      },
+    );
+
     test("functions sharing an imported identifier", () => {
       const code = dedent`
         import { sharedMessage } from "./messages";

packages/react-router-dev/vite/route-chunks.ts

@@ -109,15 +109,26 @@ function getExportDependencies(
           ...getTopLevelStatementsForPaths(identifiers),
         ]);
 
-        // We also keep track of non-import statements since import statements
-        // get more fine-grained filtering, meaning that we often need to
-        // exclude import statements in our chunking logic.
+        // We also keep track of non-module statements since most import/export
+        // statements get more fine-grained filtering. Exported function and
+        // class declarations are atomic, so keep them for shared code detection.
         let topLevelNonModuleStatements = new Set(
-          Array.from(topLevelStatements).filter(
-            (statement) =>
-              !t.isImportDeclaration(statement) &&
-              !t.isExportDeclaration(statement),
-          ),
+          Array.from(topLevelStatements).filter((statement) => {
+            if (t.isImportDeclaration(statement)) {
+              return false;
+            }
+
+            if (!t.isExportDeclaration(statement)) {
+              return true;
+            }
+
+            return (
+              (t.isExportNamedDeclaration(statement) ||
+                t.isExportDefaultDeclaration(statement)) &&
+              (t.isFunctionDeclaration(statement.declaration) ||
+                t.isClassDeclaration(statement.declaration))
+            );
+          }),
         );
 
         // We keep track of imported identifiers for each export since we