Merge branch 'dev' into feature/client-data

Author: brophdawg11

.changeset/rude-keys-heal.md

@@ -0,0 +1,15 @@
+---
+"@remix-run/dev": patch
+---
+
+Vite: Preserve names for exports from .client imports
+
+Unlike `.server` modules, the main idea is not to prevent code from leaking into the server build
+since the client build is already public. Rather, the goal is to isolate the SSR render from client-only code.
+Routes need to import code from `.client` modules without compilation failing and then rely on runtime checks
+to determine if the code is running on the server or client.
+
+Replacing `.client` modules with empty modules would cause the build to fail as ESM named imports are statically analyzed.
+So instead, we preserve the named export but replace each exported value with an empty object.
+That way, the import is valid at build time and the standard runtime checks can be used to determine if then
+code is running on the server or client.

.changeset/sour-stingrays-roll.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/serve": patch
+---
+
+Fix source map loading when file has `?t=timestamp` suffix (rebuilds)

docs/future/vite.md

@@ -641,13 +641,37 @@ With Vite, Remix gets stricter about which exports are allowed from your route m
 Previously, Remix allowed user-defined exports from routes.
 The Remix compiler would then rely on treeshaking to remove any code only intended for use on the server from the client bundle.
 
-In contrast, Vite processes each module in isolation during development, so cross-module treeshaking is not possible.
-You should already be separating server-only code into `.server` files or directories, so treeshaking isn't needed for those modules.
+```ts filename=app/routes/super-cool.tsx
+// `loader`: always server-only, remove from client bundle 👍
+export const loader = () => {};
+
+// `default`: always client-safe, keep `default` in client bundle 👍
+export default function SuperCool() {}
+
+// User-defined export
+export const mySuperCoolThing = () => {
+  /*
+  Client-safe or server-only? Depends on what code is in here... 🤷
+  Rely on treeshaking to remove from client bundle if it depends on server-only code.
+  */
+};
+```
+
+In contrast, Vite processes each module in isolation during development, so relying on cross-module treeshaking is not an option.
+For most modules, you should already be using `.server` files or directories to isolate server-only code.
 But routes are a special case since they intentionally blend client and server code.
 Remix knows that exports like `loader`, `action`, `headers`, etc. are server-only, so it can safely remove them from the client bundle.
 But there's no way to know when looking at a single route module in isolation whether user-defined exports are server-only.
 That's why Remix's Vite plugin is stricter about which exports are allowed from your route modules.
 
+```ts filename=app/routes/super-cool.tsx
+export const loader = () => {}; // server-only 👍
+export default function SuperCool() {} // client-safe 👍
+
+// Need to decide whether this is client-safe or server-only without any other information 😬
+export const mySuperCoolThing = () => {};
+```
+
 In fact, we'd rather not rely on treeshaking for correctness at all.
 If tomorrow you or your coworker accidentally imports something you _thought_ was client-safe,
 treeshaking will no longer exclude that from your client bundle and you might end up with server code in your app!
@@ -676,15 +700,14 @@ In short, Vite made us eat our veggies, but turns out they were delicious all al
 For example, here's a route with a user-defined export called `mySuperCoolThing`:
 
 ```ts filename=app/routes/super-cool.tsx
-// ❌ This isn't a Remix-specific route export, just something I made up
-export const mySuperCoolThing =
-  "Some value I wanted to colocate with my route!";
-
 // ✅ This is a valid Remix route export, so it's fine
 export const loader = () => {};
 
 // ✅ This is also a valid Remix route export
 export default function SuperCool() {}
+
+// ❌ This isn't a Remix-specific route export, just something I made up
+export const mySuperCoolThing = () => {};
 ```
 
 One option is to colocate your route and related utilities in the same directory if your routing convention allows it.
@@ -698,8 +721,7 @@ export default function SuperCool() {}
 
 ```ts filename=app/routes/super-cool/utils.ts
 // If this was server-only code, I'd rename this file to "utils.server.ts"
-export const mySuperCoolThing =
-  "Some value I wanted to colocate with my route!";
+export const mySuperCoolThing = () => {};
 ```
 
 ## Troubleshooting

integration/helpers/vite.ts

@@ -9,6 +9,8 @@ import resolveBin from "resolve-bin";
 import stripIndent from "strip-indent";
 import waitOn from "wait-on";
 import getPort from "get-port";
+import shell from "shelljs";
+import glob from "glob";
 
 const __dirname = url.fileURLToPath(new URL(".", import.meta.url));
 
@@ -249,3 +251,17 @@ export function createEditor(projectDir: string) {
     await fs.writeFile(filepath, transform(contents), "utf8");
   };
 }
+
+export function grep(cwd: string, pattern: RegExp): string[] {
+  let assetFiles = glob.sync("**/*.@(js|jsx|ts|tsx)", {
+    cwd,
+    absolute: true,
+  });
+
+  let lines = shell
+    .grep("-l", pattern, assetFiles)
+    .stdout.trim()
+    .split("\n")
+    .filter((line) => line.length > 0);
+  return lines;
+}

integration/vite-dot-client-test.ts

@@ -0,0 +1,48 @@
+import * as path from "node:path";
+import { test, expect } from "@playwright/test";
+
+import { createProject, grep, viteBuild } from "./helpers/vite.js";
+
+let files = {
+  "app/utils.client.ts": String.raw`
+    export const dotClientFile = "CLIENT_ONLY_FILE";
+    export default dotClientFile;
+  `,
+  "app/.client/utils.ts": String.raw`
+    export const dotClientDir = "CLIENT_ONLY_DIR";
+    export default dotClientDir;
+  `,
+};
+
+test("Vite / client code excluded from server bundle", async () => {
+  let cwd = await createProject({
+    ...files,
+    "app/routes/dot-client-imports.tsx": String.raw`
+      import { dotClientFile } from "../utils.client";
+      import { dotClientDir } from "../.client/utils";
+
+      export default function() {
+        const [mounted, setMounted] = useState(false);
+
+        useEffect(() => {
+          setMounted(true);
+        }, []);
+
+        return (
+          <>
+            <h2>Index</h2>
+            <p>{mounted ? dotClientFile + dotClientDir : ""}</p>
+          </>
+        );
+      }
+    `,
+  });
+  let [client, server] = viteBuild({ cwd });
+  expect(client.status).toBe(0);
+  expect(server.status).toBe(0);
+  let lines = grep(
+    path.join(cwd, "build/server"),
+    /CLIENT_ONLY_FILE|CLIENT_ONLY_DIR/
+  );
+  expect(lines).toHaveLength(0);
+});

integration/vite-dot-server-test.ts

@@ -1,9 +1,7 @@
 import * as path from "node:path";
 import { test, expect } from "@playwright/test";
-import shell from "shelljs";
-import glob from "glob";
 
-import { createProject, viteBuild } from "./helpers/vite.js";
+import { createProject, grep, viteBuild } from "./helpers/vite.js";
 
 let files = {
   "app/utils.server.ts": String.raw`
@@ -16,7 +14,7 @@ let files = {
   `,
 };
 
-test("Vite /  .server file / named import in client fails with expected error", async () => {
+test("Vite / .server file / named import in client fails with expected error", async () => {
   let cwd = await createProject({
     ...files,
     "app/routes/fail-server-file-in-client.tsx": String.raw`
@@ -35,7 +33,7 @@ test("Vite /  .server file / named import in client fails with expected error",
   );
 });
 
-test("Vite /  .server file / namespace import in client fails with expected error", async () => {
+test("Vite / .server file / namespace import in client fails with expected error", async () => {
   let cwd = await createProject({
     ...files,
     "app/routes/fail-server-file-in-client.tsx": String.raw`
@@ -126,54 +124,75 @@ test("Vite / .server dir / default import in client fails with expected error",
   expect(stderr).toMatch(`"default" is not exported by "app/.server/utils.ts"`);
 });
 
+test("Vite / `handle` with dynamic imports as an escape hatch for server-only code", async () => {
+  let cwd = await createProject({
+    ...files,
+    "app/routes/handle-server-only.tsx": String.raw`
+      export const handle = {
+        // Sharp knife alert: you probably should avoid doing this, but you can!
+        serverOnlyEscapeHatch: async () => {
+          let { dotServerFile } = await import("~/utils.server");
+          let dotServerDir = await import("~/.server/utils");
+          return { dotServerFile, dotServerDir };
+        }
+      }
+
+      export default function() {
+        return <h1>This should work</h1>
+      }
+    `,
+  });
+  let [client, server] = viteBuild({ cwd });
+  expect(client.status).toBe(0);
+  expect(server.status).toBe(0);
+
+  let lines = grep(
+    path.join(cwd, "build/client"),
+    /SERVER_ONLY_FILE|SERVER_ONLY_DIR/
+  );
+  expect(lines).toHaveLength(0);
+});
+
 test("Vite / dead-code elimination for server exports", async () => {
   let cwd = await createProject({
     ...files,
     "app/routes/remove-server-exports-and-dce.tsx": String.raw`
-        import fs from "node:fs";
-        import { json } from "@remix-run/node";
-        import { useLoaderData } from "@remix-run/react";
+      import fs from "node:fs";
+      import { json } from "@remix-run/node";
+      import { useLoaderData } from "@remix-run/react";
 
-        import { dotServerFile } from "../utils.server";
-        import { dotServerDir } from "../.server/utils";
+      import { dotServerFile } from "../utils.server";
+      import { dotServerDir } from "../.server/utils";
 
-        export const loader = () => {
-          let contents = fs.readFileSync("blah");
-          let data = dotServerFile + dotServerDir + serverOnly + contents;
-          return json({ data });
-        }
+      export const loader = () => {
+        let contents = fs.readFileSync("blah");
+        let data = dotServerFile + dotServerDir + serverOnly + contents;
+        return json({ data });
+      }
 
-        export const action = () => {
-          console.log(dotServerFile, dotServerDir, serverOnly);
-          return null;
-        }
+      export const action = () => {
+        console.log(dotServerFile, dotServerDir, serverOnly);
+        return null;
+      }
 
-        export default function() {
-          let { data } = useLoaderData<typeof loader>();
-          return (
-            <>
-              <h2>Index</h2>
-              <p>{data}</p>
-            </>
-          );
-        }
-      `,
+      export default function() {
+        let { data } = useLoaderData<typeof loader>();
+        return (
+          <>
+            <h2>Index</h2>
+            <p>{data}</p>
+          </>
+        );
+      }
+    `,
   });
-  let client = viteBuild({ cwd })[0];
+  let [client, server] = viteBuild({ cwd });
   expect(client.status).toBe(0);
+  expect(server.status).toBe(0);
 
-  // detect client asset files
-  let assetFiles = glob.sync("**/*.@(js|jsx|ts|tsx)", {
-    cwd: path.join(cwd, "build/client"),
-    absolute: true,
-  });
-
-  // grep for server-only values in client assets
-  let result = shell
-    .grep("-l", /SERVER_ONLY_FILE|SERVER_ONLY_DIR|node:fs/, assetFiles)
-    .stdout.trim()
-    .split("\n")
-    .filter((line) => line.length > 0);
-
-  expect(result).toHaveLength(0);
+  let lines = grep(
+    path.join(cwd, "build/client"),
+    /SERVER_ONLY_FILE|SERVER_ONLY_DIR|node:fs/
+  );
+  expect(lines).toHaveLength(0);
 });

packages/remix-dev/vite/plugin.ts

@@ -945,14 +945,21 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
     },
     {
       name: "remix-empty-client-modules",
-      enforce: "pre",
-      async transform(_code, id, options) {
+      enforce: "post",
+      async transform(code, id, options) {
         if (!options?.ssr) return;
         let clientFileRE = /\.client(\.[cm]?[jt]sx?)?$/;
         let clientDirRE = /\/\.client\//;
         if (clientFileRE.test(id) || clientDirRE.test(id)) {
+          let exports = esModuleLexer(code)[1];
           return {
-            code: "export {}",
+            code: exports
+              .map(({ n: name }) =>
+                name === "default"
+                  ? "export default {};"
+                  : `export const ${name} = {};`
+              )
+              .join("\n"),
             map: null,
           };
         }

packages/remix-serve/cli.ts

@@ -18,7 +18,19 @@ import getPort from "get-port";
 
 process.env.NODE_ENV = process.env.NODE_ENV ?? "production";
 
-sourceMapSupport.install();
+sourceMapSupport.install({
+  retrieveSourceMap: function (source) {
+    // get source file without the `file://` prefix or `?t=...` suffix
+    let match = source.match(/^file:\/\/(.*)\?t=[.\d]+$/);
+    if (match) {
+      return {
+        url: source,
+        map: fs.readFileSync(`${match[1]}.map`, "utf8"),
+      };
+    }
+    return null;
+  },
+});
 installGlobals();
 
 run();

templates/express/server.js

@@ -9,7 +9,19 @@ import express from "express";
 import morgan from "morgan";
 import sourceMapSupport from "source-map-support";
 
-sourceMapSupport.install();
+sourceMapSupport.install({
+  retrieveSourceMap: function (source) {
+    // get source file without the `file://` prefix or `?t=...` suffix
+    const match = source.match(/^file:\/\/(.*)\?t=[.\d]+$/);
+    if (match) {
+      return {
+        url: source,
+        map: fs.readFileSync(`${match[1]}.map`, "utf8"),
+      };
+    }
+    return null;
+  },
+});
 installGlobals();
 
 /** @typedef {import('@remix-run/node').ServerBuild} ServerBuild */