Repository: remix-run/remix
Pull request #6306 (2023-05-04 18:00:26)

refactor(dev): revert serving assets from dev server
When sending HMR updates, the assumption was that serving assets from dev server would allow those updates to be sent immediately, without waiting for the app server to restart. But this isn't the case (e.g. if you navigate to another page that has a loader and the app server is rebooting, that loader fetch will fail).

---

Idea moving forward is:
1) If you don't mind a bit of downtime from your app server during dev, then you can rely on Remix restarts. but you'll need to wait for app server to reboot before you hard refresh or click buttons that trigger actions, or navigate to different pages that need loaders.

2) if that amount of downtime is too much for you, using chokidar and purgeRequireCache w/ --no-restart and that will keep you zipping along with an always-up server.

Note: during rebuild there is no downtime, its only after rebuild but before app server restart that there is some downtime with (1) 
^not serving assets from dev server also means there's less of a difference b/w dev and prod, which is nice. plus we don't have to use dataurls to handle svgs if we do that.


changeset-bot[bot]: ###  ⚠️  No Changeset found

Latest commit: 9133202ba268f24e31a1db6acb68c724a4c11a7f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. **If these changes should result in a version bump, you need to add a changeset.**

<details><summary>This PR includes no changesets</summary>

  When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

</details>

[Click here to learn what changesets are, and how to add one](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add a changeset to this PR](https://github.com/remix-run/remix/new/pedro/refactor-do-not-server-assets-from-dev-server?filename=.changeset/warm-hounds-mate.md&value=---%0A%22remix%22%3A%20patch%0A%22%40remix-run%2Fdev%22%3A%20patch%0A---%0A%0Arefactor(dev)%3A%20revert%20serving%20assets%20from%20dev%20server%0A)



Patch:
diff --git a/.changeset/ninety-flies-allow.md b/.changeset/ninety-flies-allow.md
deleted file mode 100644
index c7787534a..000000000
--- a/.changeset/ninety-flies-allow.md
+++ /dev/null
@@ -1,7 +0,0 @@
----
-"@remix-run/dev": patch
----
-
-- Fix public asset serving for non-Remix assets in the new dev server
-- Add `--public-directory` / `unstable_dev.publicDirectory` option for configuring local directory for serving non-Remix assets like fonts
-- Fix `svg` serving for new dev server
diff --git a/packages/remix-dev/cli/commands.ts b/packages/remix-dev/cli/commands.ts
index ece1033fc..252449b9b 100644
--- a/packages/remix-dev/cli/commands.ts
+++ b/packages/remix-dev/cli/commands.ts
@@ -224,7 +224,6 @@ export async function dev(
     httpHost?: string;
     httpPort?: number;
     restart?: boolean;
-    publicDirectory?: string;
     websocketPort?: number;
   } = {}
 ) {
@@ -516,7 +515,6 @@ let resolveDevBuild = async (
 
 type DevServeFlags = DevBuildFlags & {
   command: string;
-  publicDirectory: string;
   restart: boolean;
 };
 let resolveDevServe = async (
@@ -555,11 +553,6 @@ let resolveDevServe = async (
     }
   }
 
-  let publicDirectory =
-    flags.publicDirectory ??
-    (dev === true ? undefined : dev.publicDirectory) ??
-    "public";
-
   let restart =
     flags.restart ?? (dev === true ? undefined : dev.restart) ?? true;
 
@@ -568,7 +561,6 @@ let resolveDevServe = async (
     httpScheme,
     httpHost,
     httpPort,
-    publicDirectory,
     websocketPort,
     restart,
   };
diff --git a/packages/remix-dev/cli/run.ts b/packages/remix-dev/cli/run.ts
index ff5000c28..c4448f122 100644
--- a/packages/remix-dev/cli/run.ts
+++ b/packages/remix-dev/cli/run.ts
@@ -48,7 +48,6 @@ ${colors.logoBlue("R")} ${colors.logoGreen("E")} ${colors.logoYellow(
     --http-host         HTTP(S) host for the dev server. Default: localhost
     --http-port         HTTP(S) port for the dev server. Default: any open port
     --no-restart        Do not restart the app server when rebuilds occur.
-    --public-directory  Path to public assets directory relative to Remix project root. Default: public
     --websocket-port    Websocket port for the dev server. Default: any open port
   \`init\` Options:
     --no-delete         Skip deleting the \`remix.init\` script
@@ -189,7 +188,6 @@ export async function run(argv: string[] = process.argv.slice(2)) {
       "--http-host": String,
       "--http-port": Number,
       "--no-restart": Boolean,
-      "--public-directory": String,
       "--websocket-port": Number,
     },
     {
@@ -227,10 +225,6 @@ export async function run(argv: string[] = process.argv.slice(2)) {
     flags.httpPort = flags["http-port"];
     delete flags["http-port"];
   }
-  if (flags["public-directory"]) {
-    flags.publicDirectory = flags["public-directory"];
-    delete flags["public-directory"];
-  }
   if (flags["websocket-port"]) {
     flags.websocketPort = flags["websocket-port"];
     delete flags["websocket-port"];
diff --git a/packages/remix-dev/compiler/js/compiler.ts b/packages/remix-dev/compiler/js/compiler.ts
index cf96fc0d4..e2bed448f 100644
--- a/packages/remix-dev/compiler/js/compiler.ts
+++ b/packages/remix-dev/compiler/js/compiler.ts
@@ -202,11 +202,7 @@ const createEsbuildConfig = (
     platform: "browser",
     format: "esm",
     external: getExternals(ctx.config),
-    loader: {
-      ...loaders,
-      // in development, use dataurls for svgs so that `<use href={mysvg} />` works across different origins
-      ".svg": ctx.options.mode === "development" ? "dataurl" : loaders[".svg"],
-    },
+    loader: loaders,
     bundle: true,
     logLevel: "silent",
     splitting: true,
diff --git a/packages/remix-dev/devServer_unstable/index.ts b/packages/remix-dev/devServer_unstable/index.ts
index 7767b7e25..05a427229 100644
--- a/packages/remix-dev/devServer_unstable/index.ts
+++ b/packages/remix-dev/devServer_unstable/index.ts
@@ -8,7 +8,7 @@ import express from "express";
 import * as Channel from "../channel";
 import { type Manifest } from "../manifest";
 import * as Compiler from "../compiler";
-import { readConfig, type RemixConfig } from "../config";
+import { type RemixConfig } from "../config";
 import { loadEnv } from "./env";
 import * as Socket from "./socket";
 import * as HMR from "./hmr";
@@ -23,19 +23,6 @@ type Origin = {
 
 let stringifyOrigin = (o: Origin) => `${o.scheme}://${o.host}:${o.port}`;
 
-let patchPublicPath = (
-  config: RemixConfig,
-  devHttpOrigin: Origin
-): RemixConfig => {
-  // set public path to point to dev server
-  // so that browser asks the dev server for assets
-  return {
-    ...config,
-    // dev server has its own origin, to `/build/` path will not cause conflicts with app server routes
-    publicPath: stringifyOrigin(devHttpOrigin) + "/build/",
-  };
-};
-
 let detectBin = async (): Promise<string> => {
   let pkgManager = detectPackageManager() ?? "npm";
   if (pkgManager === "npm") {
@@ -54,7 +41,6 @@ export let serve = async (
     httpScheme: string;
     httpHost: string;
     httpPort: number;
-    publicDirectory: string;
     websocketPort: number;
     restart: boolean;
   }
@@ -119,7 +105,7 @@ export let serve = async (
 
   let dispose = await Compiler.watch(
     {
-      config: patchPublicPath(initialConfig, httpOrigin),
+      config: initialConfig,
       options: {
         mode: "development",
         sourcemap: true,
@@ -129,10 +115,6 @@ export let serve = async (
       },
     },
     {
-      reloadConfig: async (root) => {
-        let config = await readConfig(root);
-        return patchPublicPath(config, httpOrigin);
-      },
       onBuildStart: (ctx) => {
         state.appReady?.err();
         clean(ctx.config);
@@ -191,20 +173,6 @@ export let serve = async (
   );
 
   let httpServer = express()
-    // statically serve built assets
-    .use((_, res, next) => {
-      res.header("Access-Control-Allow-Origin", "*");
-      next();
-    })
-    .use(
-      "/build",
-      express.static(initialConfig.assetsBuildDirectory, {
-        immutable: true,
-        maxAge: "1y",
-      })
-    )
-    .use(express.static(options.publicDirectory, { maxAge: "1h" }))
-
     // handle `broadcastDevReady` messages
     .use(express.json())
     .post("/ping", (req, res) => {


Test patch:
diff --git a/packages/remix-dev/__tests__/cli-test.ts b/packages/remix-dev/__tests__/cli-test.ts
index 59e5dcb93..ba57c1006 100644
--- a/packages/remix-dev/__tests__/cli-test.ts
+++ b/packages/remix-dev/__tests__/cli-test.ts
@@ -122,7 +122,6 @@ describe("remix CLI", () => {
             --http-host         HTTP(S) host for the dev server. Default: localhost
             --http-port         HTTP(S) port for the dev server. Default: any open port
             --no-restart        Do not restart the app server when rebuilds occur.
-            --public-directory  Path to public assets directory relative to Remix project root. Default: public
             --websocket-port    Websocket port for the dev server. Default: any open port
           \`init\` Options:
             --no-delete         Skip deleting the \`remix.init\` script