fix: port solid-start route treeshake fixes (#2238, #2249)

.changeset/olive-pugs-shave.md

@@ -0,0 +1,5 @@
+---
+"filesystem-routing": patch
+---
+
+keep TypeScript namespace members in route files during production builds

.changeset/tidy-donuts-sing.md

@@ -0,0 +1,5 @@
+---
+"filesystem-routing": patch
+---
+
+keep route module ids ending in a real extension so ecosystem plugins match, and name route chunks after their file rather than their pick id

src/vite/index.ts

@@ -8,11 +8,11 @@ import { BaseFileSystemRouter, normalizePath } from "../router.ts";
 import { buildRouteTree, type RouteTreeEntry } from "../tree.ts";
 import { DEFAULT_EXTENSIONS, moduleId } from "./constants.ts";
 import { fileSystemWatcher } from "./fs-watcher.ts";
-import { treeShake } from "./tree-shake.ts";
+import { sanitizeChunkFileName, toPickId, treeShake } from "./tree-shake.ts";
 import { serializeTypes } from "./types.ts";
 
 export { DEFAULT_EXTENSIONS, moduleId };
-export { treeShake } from "./tree-shake.ts";
+export { sanitizeChunkFileName, toPickId, treeShake } from "./tree-shake.ts";
 export { fileSystemWatcher } from "./fs-watcher.ts";
 
 export interface FileRoutesOptions extends Pick<
@@ -131,21 +131,28 @@ export function fileRoutes(options: FileRoutesOptions = {}): PluginOption[] {
   }
 
   /** The id a route module ref is loaded from: its source plus its picks. */
-  const toModuleId = (ref: ModuleRef) =>
-    `${ref.src}?${ref.pick.map(pick => `pick=${pick}`).join("&")}`;
+  const toModuleId = (ref: ModuleRef) => toPickId(ref.src, ref.pick);
 
   return [
     {
       name: "filesystem-routing",
       enforce: "pre",
       config() {
-        // Packages importing the virtual module (which only this plugin can
-        // resolve) must stay out of esbuild prebundling.
-        if (!options.optimizeDepsExclude?.length) return;
         return {
-          optimizeDeps: {
-            exclude: options.optimizeDepsExclude
-          }
+          build: {
+            rollupOptions: {
+              output: {
+                // Keeps route chunks named after their file rather than after
+                // the `?pick=...` id that addresses them. See toPickId.
+                sanitizeFileName: sanitizeChunkFileName
+              }
+            }
+          },
+          // Packages importing the virtual module (which only this plugin can
+          // resolve) must stay out of esbuild prebundling.
+          ...(options.optimizeDepsExclude?.length
+            ? { optimizeDeps: { exclude: options.optimizeDepsExclude } }
+            : {})
         };
       },
       configResolved(config) {

src/vite/tree-shake.ts

@@ -5,11 +5,61 @@
 import type * as Babel from "@babel/core";
 import type { NodePath, PluginObj, PluginPass } from "@babel/core";
 import type { Binding } from "@babel/traverse";
-import { basename } from "node:path";
+import { basename, extname } from "node:path";
 
 type Identifier = Babel.types.Identifier;
 import type { Plugin } from "vite";
 
+const PICKABLE_EXTENSIONS = ["js", "jsx", "ts", "tsx"];
+
+/**
+ * Builds the module id used to import a subset of a route file's exports.
+ *
+ * The `pick` list has to live in the query so that the same file can be
+ * instantiated once per export subset (the client picks `default`/`$css`,
+ * the server picks its HTTP handlers). That query would otherwise leave the
+ * id ending in `?pick=GET`, which silently excludes route files from any
+ * plugin whose filter is anchored on the file extension -- a very common
+ * default, e.g. unplugin-macros' `/\.[cm]?[jt]sx?$/`.
+ *
+ * The trailing `lang.<ext>` marker is the same convention Vue SFCs use
+ * (`?vue&type=script&lang.ts`) and puts a real extension back at the end of
+ * the id, so those filters match again.
+ *
+ * @see https://github.com/solidjs/solid-start/issues/1918
+ */
+export function toPickId(src: string, pick: string[]): string {
+  const query = pick.map(p => `pick=${p}`).join("&");
+  const ext = extname(src).slice(1);
+  return PICKABLE_EXTENSIONS.includes(ext) ? `${src}?${query}&lang.${ext}` : `${src}?${query}`;
+}
+
+/**
+ * Matches the tail that {@link toPickId} appends, as it appears in a chunk
+ * name. Bundlers derive a chunk name from the module id by taking the
+ * basename minus its extension, so `index.tsx?pick=default&pick=$css&lang.tsx`
+ * arrives here as `index.tsx?pick=default&pick=$css&lang`.
+ */
+const PICK_CHUNK_NAME_RE = /\.[cm]?[jt]sx?\?pick=[^?]*&lang$/;
+
+const INVALID_CHAR_RE = /[\u0000-\u001F"#$&*+,:;<=>?[\]^`{|}\u007F]/g;
+const DRIVE_LETTER_RE = /^[a-z]:/i;
+
+/**
+ * Rollup's default chunk name sanitizer, plus a pass that drops the
+ * `?pick=...&lang` tail so route chunks keep the short, stable filenames they
+ * had before the `lang` marker was introduced (`index-<hash>.js`, not
+ * `index.tsx_pick_default_pick__css_lang-<hash>.js`). Client chunk filenames
+ * are public URLs, so this is not purely cosmetic.
+ *
+ * @see https://github.com/rollup/rollup/blob/master/src/utils/sanitizeFileName.ts
+ */
+export function sanitizeChunkFileName(name: string): string {
+  const stripped = name.replace(PICK_CHUNK_NAME_RE, "");
+  const driveLetter = DRIVE_LETTER_RE.exec(stripped)?.[0] ?? "";
+  return driveLetter + stripped.slice(driveLetter.length).replace(INVALID_CHAR_RE, "_");
+}
+
 type State = Omit<PluginPass, "opts"> & {
   opts: { pick: string[] };
   refs: Set<any>;
@@ -148,6 +198,11 @@ function treeShakeTransform({ types: t }: typeof Babel): PluginObj<State> {
                 if (!state.opts.pick) {
                   return;
                 }
+                // Only module-level exports are route exports; `export` inside a
+                // TS namespace/module block is unrelated to the pick list.
+                if (!exportNamedPath.parentPath.isProgram()) {
+                  return;
+                }
                 const specifiers = exportNamedPath.get("specifiers");
                 if (specifiers.length) {
                   specifiers.forEach(s => {

test/tree-shake.spec.ts

@@ -1,14 +1,48 @@
 import { describe, expect, it } from "vitest";
 
-import { treeShake } from "../src/vite/tree-shake.ts";
+import { sanitizeChunkFileName, toPickId, treeShake } from "../src/vite/tree-shake.ts";
 
 async function shake(code: string, pick: string[]) {
   const plugin = treeShake() as any;
-  const id = `/routes/route.ts?${pick.map(p => `pick=${p}`).join("&")}`;
+  const id = toPickId("/routes/route.ts", pick);
   const result = await plugin.transform(code, id);
   return result?.code as string | undefined;
 }
 
+// https://github.com/solidjs/solid-start/issues/1918
+describe("toPickId", () => {
+  it("ends the id with the source extension so extension filters still match", () => {
+    const id = toPickId("/routes/route.ts", ["GET"]);
+
+    expect(id).toBe("/routes/route.ts?pick=GET&lang.ts");
+    // the default filter used by unplugin-macros and many other plugins
+    expect(id).toMatch(/\.[cm]?[jt]sx?$/);
+  });
+
+  it("keeps every picked export in the query", () => {
+    expect(toPickId("/routes/route.tsx", ["default", "$css"])).toBe(
+      "/routes/route.tsx?pick=default&pick=$css&lang.tsx"
+    );
+  });
+
+  it("leaves non-script sources alone", () => {
+    expect(toPickId("/routes/route.mdx", ["default"])).toBe("/routes/route.mdx?pick=default");
+  });
+});
+
+describe("sanitizeChunkFileName", () => {
+  it("drops the pick query so route chunks keep their file-based name", () => {
+    expect(sanitizeChunkFileName("index.tsx?pick=default&pick=$css&lang")).toBe("index");
+    expect(sanitizeChunkFileName("macro.ts?pick=GET&lang")).toBe("macro");
+  });
+
+  it("still sanitizes the rest of the name like the bundler does", () => {
+    expect(sanitizeChunkFileName("[...404].tsx?pick=default&pick=$css&lang")).toBe("_...404_");
+    expect(sanitizeChunkFileName("entry-client")).toBe("entry-client");
+    expect(sanitizeChunkFileName("_libs/solid-js")).toBe("_libs/solid-js");
+  });
+});
+
 describe("treeShake", () => {
   it("keeps only the picked export", async () => {
     const code = `
@@ -286,4 +320,19 @@ describe("treeShake", () => {
     expect(output).not.toContain("initializeSecret");
     expect(output).toContain("export const GET");
   });
+
+  // https://github.com/solidjs/solid-start/pull/2238
+  it("keeps exports inside a TS namespace that a picked export references", async () => {
+    const code = `
+      namespace Data {
+        export const value = "ok";
+      }
+      export const GET = () => Data.value;
+    `;
+
+    const output = await shake(code, ["GET"]);
+
+    expect(output).toContain("export const value");
+    expect(output).toContain("export const GET");
+  });
 });

test/vite.spec.ts

@@ -49,10 +49,13 @@ describe("fileRoutes vite plugin", () => {
     const code = await loadVirtualModule(root);
 
     expect(code).toContain("export default routes;");
-    // lazy refs become code-split dynamic imports picking component exports
-    expect(code).toMatch(/import\('[^']*index\.tsx\?pick=default&pick=\$css'\)/);
+    // lazy refs become code-split dynamic imports picking component exports;
+    // the id ends in a real extension so extension-filtered plugins match it
+    expect(code).toMatch(/import\('[^']*index\.tsx\?pick=default&pick=\$css&lang\.tsx'\)/);
     // eager refs become static imports of the route config
-    expect(code).toMatch(/import { route as routeData0 } from '[^']*\[id\]\.tsx\?pick=route';/);
+    expect(code).toMatch(
+      /import { route as routeData0 } from '[^']*\[id\]\.tsx\?pick=route&lang\.tsx';/
+    );
     expect(code).toContain(`"path":"/blog/:id"`);
     expect(code).toContain(`'route': routeData0`);
   });
@@ -73,7 +76,7 @@ describe("fileRoutes vite plugin", () => {
       /export const pageRoutes = \[\{ \.\.\.route\d, id: "\/\(app\)", path: "\/", children: \[\{ \.\.\.route\d, id: "\/dashboard", path: "\/dashboard" \}\] \}\]/
     );
     // entries are referenced, not copied, so refs are emitted exactly once
-    expect(code.match(/pick=default&pick=\$css'\)/g)?.length).toBe(2);
+    expect(code.match(/pick=default&pick=\$css&lang\.tsx'\)/g)?.length).toBe(2);
   });
 
   it("resolves only the virtual module id", async () => {
@@ -92,12 +95,19 @@ describe("fileRoutes vite plugin", () => {
   it("excludes packages importing the virtual module from prebundling on request", () => {
     // adapters take the manifest as an argument, so nothing is excluded by default
     const [defaults] = fileRoutes() as any[];
-    expect(defaults.config()).toBeUndefined();
+    expect(defaults.config().optimizeDeps).toBeUndefined();
 
     const [custom] = fileRoutes({ optimizeDepsExclude: ["some-adapter"] }) as any[];
     expect(custom.config().optimizeDeps.exclude).toEqual(["some-adapter"]);
   });
 
+  it("names route chunks after their file, not their pick id", () => {
+    const [plugin] = fileRoutes() as any[];
+    const sanitize = plugin.config().build.rollupOptions.output.sanitizeFileName;
+
+    expect(sanitize("index.tsx?pick=default&pick=$css&lang")).toBe("index");
+  });
+
   describe("types", () => {
     it("declares the manifest as a literal tuple", async () => {
       const root = createRouteTree({
@@ -192,8 +202,10 @@ describe("fileRoutes vite plugin", () => {
 
       expect(input).toHaveLength(2);
       expect(input.every(id => id.includes("?pick=default&pick=$css"))).toBe(true);
+      // ids end in a real extension so extension-filtered plugins match them
+      expect(input.every(id => id.endsWith("&lang.tsx"))).toBe(true);
       // `$$route` refs are inlined into the manifest, so they are not entries
-      expect(input.some(id => id.endsWith("?pick=route"))).toBe(false);
+      expect(input.some(id => id.includes("?pick=route"))).toBe(false);
     });
 
     it("only contributes inputs to the named environments, on build", async () => {