perf(remix-server-runtime): Performance improvements for large apps (#…

…4748) Co-authored-by: Matt Brophy <matt@brophy.org>
remix-run · Feb 2, 2023 · 42b0a4f · 42b0a4f
1 parent 7f3c92b
commit 42b0a4f
Show file tree

Hide file tree

Showing 3 changed files with 178 additions and 124 deletions.
diff --git a/.changeset/many-pans-attack.md b/.changeset/many-pans-attack.md
@@ -0,0 +1,6 @@
+---
+"@remix-run/react": patch
+"@remix-run/server-runtime": patch
+---
+
+Improve efficiency of route manifest->tree transformation
diff --git a/packages/remix-react/routes.tsx b/packages/remix-react/routes.tsx
@@ -45,87 +45,109 @@ export interface EntryRoute extends Route {
   parentId?: string;
 }
 
+// Create a map of routes by parentId to use recursively instead of
+// repeatedly filtering the manifest.
+function groupRoutesByParentId(manifest: RouteManifest<EntryRoute>) {
+  let routes: Record<string, Omit<EntryRoute, "children">[]> = {};
+
+  Object.values(manifest).forEach((route) => {
+    let parentId = route.parentId || "";
+    if (!routes[parentId]) {
+      routes[parentId] = [];
+    }
+    routes[parentId].push(route);
+  });
+
+  return routes;
+}
+
 export function createServerRoutes(
   manifest: RouteManifest<EntryRoute>,
   routeModules: RouteModules,
   future: FutureConfig,
-  parentId?: string
+  parentId: string = "",
+  routesByParentId: Record<
+    string,
+    Omit<EntryRoute, "children">[]
+  > = groupRoutesByParentId(manifest)
 ): DataRouteObject[] {
-  return Object.values(manifest)
-    .filter((route) => route.parentId === parentId)
-    .map((route) => {
-      let hasErrorBoundary =
-        future.v2_errorBoundary === true
-          ? route.id === "root" || route.hasErrorBoundary
-          : route.id === "root" ||
-            route.hasCatchBoundary ||
-            route.hasErrorBoundary;
-      let dataRoute: DataRouteObject = {
-        caseSensitive: route.caseSensitive,
-        element: <RemixRoute id={route.id} />,
-        errorElement: hasErrorBoundary ? (
-          <RemixRouteError id={route.id} />
-        ) : undefined,
-        id: route.id,
-        index: route.index,
-        path: route.path,
-        handle: routeModules[route.id].handle,
-        // Note: we don't need loader/action/shouldRevalidate on these routes
-        // since they're for a static render
-      };
-
-      let children = createServerRoutes(
-        manifest,
-        routeModules,
-        future,
-        route.id
-      );
-      if (children.length > 0) dataRoute.children = children;
-      return dataRoute;
-    });
+  return (routesByParentId[parentId] || []).map((route) => {
+    let hasErrorBoundary =
+      future.v2_errorBoundary === true
+        ? route.id === "root" || route.hasErrorBoundary
+        : route.id === "root" ||
+          route.hasCatchBoundary ||
+          route.hasErrorBoundary;
+    let dataRoute: DataRouteObject = {
+      caseSensitive: route.caseSensitive,
+      element: <RemixRoute id={route.id} />,
+      errorElement: hasErrorBoundary ? (
+        <RemixRouteError id={route.id} />
+      ) : undefined,
+      id: route.id,
+      index: route.index,
+      path: route.path,
+      handle: routeModules[route.id].handle,
+      // Note: we don't need loader/action/shouldRevalidate on these routes
+      // since they're for a static render
+    };
+
+    let children = createServerRoutes(
+      manifest,
+      routeModules,
+      future,
+      route.id,
+      routesByParentId
+    );
+    if (children.length > 0) dataRoute.children = children;
+    return dataRoute;
+  });
 }
 
 export function createClientRoutes(
   manifest: RouteManifest<EntryRoute>,
   routeModulesCache: RouteModules,
   future: FutureConfig,
-  parentId?: string
+  parentId: string = "",
+  routesByParentId: Record<
+    string,
+    Omit<EntryRoute, "children">[]
+  > = groupRoutesByParentId(manifest)
 ): DataRouteObject[] {
-  return Object.values(manifest)
-    .filter((entryRoute) => entryRoute.parentId === parentId)
-    .map((route) => {
-      let hasErrorBoundary =
-        future.v2_errorBoundary === true
-          ? route.id === "root" || route.hasErrorBoundary
-          : route.id === "root" ||
-            route.hasCatchBoundary ||
-            route.hasErrorBoundary;
-
-      let dataRoute: DataRouteObject = {
-        caseSensitive: route.caseSensitive,
-        element: <RemixRoute id={route.id} />,
-        errorElement: hasErrorBoundary ? (
-          <RemixRouteError id={route.id} />
-        ) : undefined,
-        id: route.id,
-        index: route.index,
-        path: route.path,
-        // handle gets added in via useMatches since we aren't guaranteed to
-        // have the route module available here
-        handle: undefined,
-        loader: createDataFunction(route, routeModulesCache, false),
-        action: createDataFunction(route, routeModulesCache, true),
-        shouldRevalidate: createShouldRevalidate(route, routeModulesCache),
-      };
-      let children = createClientRoutes(
-        manifest,
-        routeModulesCache,
-        future,
-        route.id
-      );
-      if (children.length > 0) dataRoute.children = children;
-      return dataRoute;
-    });
+  return (routesByParentId[parentId] || []).map((route) => {
+    let hasErrorBoundary =
+      future.v2_errorBoundary === true
+        ? route.id === "root" || route.hasErrorBoundary
+        : route.id === "root" ||
+          route.hasCatchBoundary ||
+          route.hasErrorBoundary;
+
+    let dataRoute: DataRouteObject = {
+      caseSensitive: route.caseSensitive,
+      element: <RemixRoute id={route.id} />,
+      errorElement: hasErrorBoundary ? (
+        <RemixRouteError id={route.id} />
+      ) : undefined,
+      id: route.id,
+      index: route.index,
+      path: route.path,
+      // handle gets added in via useMatches since we aren't guaranteed to
+      // have the route module available here
+      handle: undefined,
+      loader: createDataFunction(route, routeModulesCache, false),
+      action: createDataFunction(route, routeModulesCache, true),
+      shouldRevalidate: createShouldRevalidate(route, routeModulesCache),
+    };
+    let children = createClientRoutes(
+      manifest,
+      routeModulesCache,
+      future,
+      route.id,
+      routesByParentId
+    );
+    if (children.length > 0) dataRoute.children = children;
+    return dataRoute;
+  });
 }
 
 function createShouldRevalidate(

diff --git a/packages/remix-server-runtime/routes.ts b/packages/remix-server-runtime/routes.ts
@@ -4,6 +4,7 @@ import type {
   LoaderFunctionArgs,
 } from "@remix-run/router";
 
+import type { AppLoadContext } from "./data";
 import { callRouteActionRR, callRouteLoaderRR } from "./data";
 import type { FutureConfig } from "./entry";
 import type { ServerRouteModule } from "./routeModules";
@@ -39,71 +40,96 @@ export interface ServerRoute extends Route {
   module: ServerRouteModule;
 }
 
+function groupRoutesByParentId(manifest: ServerRouteManifest) {
+  let routes: Record<string, Omit<ServerRoute, "children">[]> = {};
+
+  Object.values(manifest).forEach((route) => {
+    let parentId = route.parentId || "";
+    if (!routes[parentId]) {
+      routes[parentId] = [];
+    }
+    routes[parentId].push(route);
+  });
+
+  return routes;
+}
+
+// Create a map of routes by parentId to use recursively instead of
+// repeatedly filtering the manifest.
 export function createRoutes(
   manifest: ServerRouteManifest,
-  parentId?: string
+  parentId: string = "",
+  routesByParentId: Record<
+    string,
+    Omit<ServerRoute, "children">[]
+  > = groupRoutesByParentId(manifest)
 ): ServerRoute[] {
-  return Object.entries(manifest)
-    .filter(([, route]) => route.parentId === parentId)
-    .map(([id, route]) => ({
-      ...route,
-      children: createRoutes(manifest, id),
-    }));
+  return (routesByParentId[parentId] || []).map((route) => ({
+    ...route,
+    children: createRoutes(manifest, route.id, routesByParentId),
+  }));
 }
 
 // Convert the Remix ServerManifest into DataRouteObject's for use with
 // createStaticHandler
 export function createStaticHandlerDataRoutes(
   manifest: ServerRouteManifest,
   future: FutureConfig,
-  parentId?: string
+  parentId: string = "",
+  routesByParentId: Record<
+    string,
+    Omit<ServerRoute, "children">[]
+  > = groupRoutesByParentId(manifest)
 ): AgnosticDataRouteObject[] {
-  return Object.values(manifest)
-    .filter((route) => route.parentId === parentId)
-    .map((route) => {
-      let hasErrorBoundary =
-        future.v2_errorBoundary === true
-          ? route.id === "root" || route.module.ErrorBoundary != null
-          : route.id === "root" ||
-            route.module.CatchBoundary != null ||
-            route.module.ErrorBoundary != null;
-      let commonRoute = {
-        // Always include root due to default boundaries
-        hasErrorBoundary,
-        id: route.id,
-        path: route.path,
-        loader: route.module.loader
-          ? (args: LoaderFunctionArgs) =>
-              callRouteLoaderRR({
-                request: args.request,
-                params: args.params,
-                loadContext: args.context,
-                loader: route.module.loader!,
-                routeId: route.id,
-              })
-          : undefined,
-        action: route.module.action
-          ? (args: ActionFunctionArgs) =>
-              callRouteActionRR({
-                request: args.request,
-                params: args.params,
-                loadContext: args.context,
-                action: route.module.action!,
-                routeId: route.id,
-              })
-          : undefined,
-        handle: route.module.handle,
-      };
+  return (routesByParentId[parentId] || []).map((route) => {
+    let hasErrorBoundary =
+      future.v2_errorBoundary === true
+        ? route.id === "root" || route.module.ErrorBoundary != null
+        : route.id === "root" ||
+          route.module.CatchBoundary != null ||
+          route.module.ErrorBoundary != null;
+    let commonRoute = {
+      // Always include root due to default boundaries
+      hasErrorBoundary,
+      id: route.id,
+      path: route.path,
+      loader: route.module.loader
+        ? (args: LoaderFunctionArgs) =>
+            callRouteLoaderRR({
+              request: args.request,
+              params: args.params,
+              loadContext: args.context,
+              loader: route.module.loader!,
+              routeId: route.id,
+            })
+        : undefined,
+      action: route.module.action
+        ? (args: ActionFunctionArgs) =>
+            callRouteActionRR({
+              request: args.request,
+              params: args.params,
+              loadContext: args.context,
+              action: route.module.action!,
+              routeId: route.id,
+            })
+        : undefined,
+      handle: route.module.handle,
+    };
 
-      return route.index
-        ? {
-            index: true,
-            ...commonRoute,
-          }
-        : {
-            caseSensitive: route.caseSensitive,
-            children: createStaticHandlerDataRoutes(manifest, future, route.id),
-            ...commonRoute,
-          };
-    });
+    return route.index
+      ? {
+          index: true,
+          ...commonRoute,
+        }
+      : {
+          caseSensitive: route.caseSensitive,
+          children: createStaticHandlerDataRoutes(
+            manifest,
+            future,
+            route.id,
+            routesByParentId
+          ),
+          ...commonRoute,
+        };
+  });
 }