Fix serverBundles issue with multiple browser manifests

remix-run · Feb 23, 2024 · d2d7f41 · d2d7f41
1 parent 287d7da
commit d2d7f41
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 41 deletions.
diff --git a/.changeset/silver-years-roll.md b/.changeset/silver-years-roll.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/dev": patch
+---
+
+Vite: Fix `serverBundles` issue where multiple browser manifests are generated
diff --git a/integration/vite-server-bundles-test.ts b/integration/vite-server-bundles-test.ts
@@ -294,6 +294,17 @@ test.describe(() => {
       expect(pageErrors).toEqual([]);
     });
 
+    test("Vite / server bundles / build / Remix browser manifest", () => {
+      let clientAssetFiles = fs.readdirSync(
+        path.join(cwd, "build", "client", "assets")
+      );
+      let manifestFiles = clientAssetFiles.filter((filename) =>
+        filename.startsWith("manifest-")
+      );
+
+      expect(manifestFiles.length).toEqual(1);
+    });
+
     test("Vite / server bundles / build / Vite manifests", () => {
       let viteManifestFiles = fs.readdirSync(path.join(cwd, "build", ".vite"));
 

diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts
@@ -26,7 +26,7 @@ import {
   type RemixConfig as ResolvedRemixEsbuildConfig,
   resolveConfig as resolveRemixEsbuildConfig,
 } from "../config";
-import { type Manifest as BrowserManifest } from "../manifest";
+import { type Manifest as RemixManifest } from "../manifest";
 import invariant from "../invariant";
 import {
   type NodeRequestHandler,
@@ -200,13 +200,13 @@ export type ServerBundleBuildConfig = {
 type RemixPluginSsrBuildContext =
   | {
       isSsrBuild: false;
-      getBrowserManifest?: never;
-      serverBundleId?: never;
+      generateRemixManifests?: never;
+      serverBundleBuildConfig?: never;
     }
   | {
       isSsrBuild: true;
-      getBrowserManifest: () => Promise<BrowserManifest>;
-      serverBundleId: string | undefined;
+      getRemixServerManifest: () => Promise<RemixManifest>;
+      serverBundleBuildConfig: ServerBundleBuildConfig | null;
     };
 
 export type RemixPluginContext = RemixPluginSsrBuildContext & {
@@ -286,7 +286,7 @@ const resolveBuildAssetPaths = (
   viteManifest: Vite.Manifest,
   entryFilePath: string,
   prependedAssetFilePaths: string[] = []
-): BrowserManifest["entry"] & { css: string[] } => {
+): RemixManifest["entry"] & { css: string[] } => {
   let entryChunk = resolveChunk(ctx, viteManifest, entryFilePath);
 
   // This is here to support prepending client entry assets to the root route
@@ -423,7 +423,9 @@ export let getServerBuildDirectory = (ctx: RemixPluginContext) =>
   path.join(
     ctx.remixConfig.buildDirectory,
     "server",
-    ...(typeof ctx.serverBundleId === "string" ? [ctx.serverBundleId] : [])
+    ...(ctx.serverBundleBuildConfig
+      ? [ctx.serverBundleBuildConfig.serverBundleId]
+      : [])
   );
 
 let getClientBuildDirectory = (remixConfig: ResolvedVitePluginConfig) =>
@@ -638,15 +640,6 @@ export const remixVitePlugin: RemixVitePlugin = (remixUserConfig = {}) => {
       serverBundles = undefined;
     }
 
-    // Get the server bundle build config injected by the Remix CLI, if present.
-    let serverBundleBuildConfig = getServerBundleBuildConfig(viteUserConfig);
-
-    // For server bundle builds, override the relevant config. This lets us run
-    // multiple server builds with each one targeting a subset of routes.
-    if (serverBundleBuildConfig) {
-      routes = serverBundleBuildConfig.routes;
-    }
-
     let remixConfig: ResolvedVitePluginConfig = deepFreeze({
       appDirectory,
       basename,
@@ -672,9 +665,9 @@ export const remixVitePlugin: RemixVitePlugin = (remixUserConfig = {}) => {
       viteConfigEnv.isSsrBuild && viteCommand === "build"
         ? {
             isSsrBuild: true,
-            getBrowserManifest: createBrowserManifestForBuild,
-            serverBundleId:
-              getServerBundleBuildConfig(viteUserConfig)?.serverBundleId,
+            getRemixServerManifest: async () =>
+              (await generateRemixManifestsForBuild()).remixServerManifest,
+            serverBundleBuildConfig: getServerBundleBuildConfig(viteUserConfig),
           }
         : { isSsrBuild: false };
 
@@ -701,13 +694,20 @@ export const remixVitePlugin: RemixVitePlugin = (remixUserConfig = {}) => {
     //   mark the old compiler as deprecated
     // - Remove `ServerBuild.mode` in v3
 
+    let routes = ctx.serverBundleBuildConfig
+      ? // For server bundle builds, the server build should only import the
+        // routes for this bundle rather than importing all routes
+        ctx.serverBundleBuildConfig.routes
+      : // Otherwise, all routes are imported as usual
+        ctx.remixConfig.routes;
+
     return `
     import * as entryServer from ${JSON.stringify(
       resolveFileUrl(ctx, ctx.entryServerFilePath)
     )};
-    ${Object.keys(ctx.remixConfig.routes)
+    ${Object.keys(routes)
       .map((key, index) => {
-        let route = ctx.remixConfig.routes[key]!;
+        let route = routes[key]!;
         return `import * as route${index} from ${JSON.stringify(
           resolveFileUrl(
             ctx,
@@ -734,9 +734,9 @@ export const remixVitePlugin: RemixVitePlugin = (remixUserConfig = {}) => {
       export const publicPath = ${JSON.stringify(ctx.remixConfig.publicPath)};
       export const entry = { module: entryServer };
       export const routes = {
-        ${Object.keys(ctx.remixConfig.routes)
+        ${Object.keys(routes)
           .map((key, index) => {
-            let route = ctx.remixConfig.routes[key]!;
+            let route = routes[key]!;
             return `${JSON.stringify(key)}: {
           id: ${JSON.stringify(route.id)},
           parentId: ${JSON.stringify(route.parentId)},
@@ -774,7 +774,10 @@ export const remixVitePlugin: RemixVitePlugin = (remixUserConfig = {}) => {
     return new Set([...cssUrlPaths, ...chunkAssetPaths]);
   };
 
-  let createBrowserManifestForBuild = async (): Promise<BrowserManifest> => {
+  let generateRemixManifestsForBuild = async (): Promise<{
+    remixBrowserManifest: RemixManifest;
+    remixServerManifest: RemixManifest;
+  }> => {
     let viteManifest = await loadViteManifest(
       getClientBuildDirectory(ctx.remixConfig)
     );
@@ -785,7 +788,8 @@ export const remixVitePlugin: RemixVitePlugin = (remixUserConfig = {}) => {
       ctx.entryClientFilePath
     );
 
-    let routes: BrowserManifest["routes"] = {};
+    let browserRoutes: RemixManifest["routes"] = {};
+    let serverRoutes: RemixManifest["routes"] = {};
 
     let routeManifestExports = await getRouteManifestModuleExports(
       viteChildCompiler,
@@ -797,7 +801,7 @@ export const remixVitePlugin: RemixVitePlugin = (remixUserConfig = {}) => {
       let sourceExports = routeManifestExports[key];
       let isRootRoute = route.parentId === undefined;
 
-      routes[key] = {
+      let routeManifestEntry = {
         id: route.id,
         parentId: route.parentId,
         path: route.path,
@@ -818,29 +822,49 @@ export const remixVitePlugin: RemixVitePlugin = (remixUserConfig = {}) => {
           isRootRoute ? [ctx.entryClientFilePath] : []
         ),
       };
+
+      browserRoutes[key] = routeManifestEntry;
+
+      let serverBundleRoutes = ctx.serverBundleBuildConfig?.routes;
+      if (!serverBundleRoutes || serverBundleRoutes[key]) {
+        serverRoutes[key] = routeManifestEntry;
+      }
     }
 
-    let fingerprintedValues = { entry, routes };
+    let fingerprintedValues = { entry, routes: browserRoutes };
     let version = getHash(JSON.stringify(fingerprintedValues), 8);
     let manifestPath = `assets/manifest-${version}.js`;
     let url = `${ctx.remixConfig.publicPath}${manifestPath}`;
     let nonFingerprintedValues = { url, version };
 
-    let manifest: BrowserManifest = {
+    let remixBrowserManifest: RemixManifest = {
       ...fingerprintedValues,
       ...nonFingerprintedValues,
     };
 
+    // Write the browser manifest to disk as part of the build process
     await writeFileSafe(
       path.join(getClientBuildDirectory(ctx.remixConfig), manifestPath),
-      `window.__remixManifest=${JSON.stringify(manifest)};`
+      `window.__remixManifest=${JSON.stringify(remixBrowserManifest)};`
     );
 
-    return manifest;
+    // The server manifest is the same as the browser manifest, except for
+    // server bundle builds which only includes routes for the current bundle,
+    // otherwise the server and client have the same routes
+    let remixServerManifest: RemixManifest = {
+      ...remixBrowserManifest,
+      routes: serverRoutes,
+    };
+
+    return {
+      remixBrowserManifest,
+      remixServerManifest,
+    };
   };
 
-  let getBrowserManifestForDev = async (): Promise<BrowserManifest> => {
-    let routes: BrowserManifest["routes"] = {};
+  // In dev, the server and browser Remix manifests are the same
+  let getRemixManifestForDev = async (): Promise<RemixManifest> => {
+    let routes: RemixManifest["routes"] = {};
 
     let routeManifestExports = await getRouteManifestModuleExports(
       viteChildCompiler,
@@ -1328,21 +1352,21 @@ export const remixVitePlugin: RemixVitePlugin = (remixUserConfig = {}) => {
             return await getServerEntry();
           }
           case VirtualModule.resolve(serverManifestId): {
-            let browserManifest = ctx.isSsrBuild
-              ? await ctx.getBrowserManifest()
-              : await getBrowserManifestForDev();
+            let remixManifest = ctx.isSsrBuild
+              ? await ctx.getRemixServerManifest()
+              : await getRemixManifestForDev();
 
-            return `export default ${jsesc(browserManifest, { es6: true })};`;
+            return `export default ${jsesc(remixManifest, { es6: true })};`;
           }
           case VirtualModule.resolve(browserManifestId): {
             if (viteCommand === "build") {
               throw new Error("This module only exists in development");
             }
 
-            let browserManifest = await getBrowserManifestForDev();
-            let browserManifestString = jsesc(browserManifest, { es6: true });
+            let remixManifest = await getRemixManifestForDev();
+            let remixManifestString = jsesc(remixManifest, { es6: true });
 
-            return `window.__remixManifest=${browserManifestString};`;
+            return `window.__remixManifest=${remixManifestString};`;
           }
         }
       },
@@ -1567,14 +1591,14 @@ export const remixVitePlugin: RemixVitePlugin = (remixUserConfig = {}) => {
       async handleHotUpdate({ server, file, modules, read }) {
         let route = getRoute(ctx.remixConfig, file);
 
-        type ManifestRoute = BrowserManifest["routes"][string];
+        type ManifestRoute = RemixManifest["routes"][string];
         type HmrEventData = { route: ManifestRoute | null };
         let hmrEventData: HmrEventData = { route: null };
 
         if (route) {
           // invalidate manifest on route exports change
           let serverManifest = (await server.ssrLoadModule(serverManifestId))
-            .default as BrowserManifest;
+            .default as RemixManifest;
 
           let oldRouteMetadata = serverManifest.routes[route.id];
           let newRouteMetadata = await getRouteMetadata(