diff --git a/.changeset/many-pans-attack.md b/.changeset/many-pans-attack.md new file mode 100644 index 00000000000..e343ff10f55 --- /dev/null +++ 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 index 78ee22b8424..230c49a46c7 100644 --- 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) { + let routes: Record[]> = {}; + + 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, routeModules: RouteModules, future: FutureConfig, - parentId?: string + parentId: string = "", + routesByParentId: Record< + string, + Omit[] + > = 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: , - errorElement: hasErrorBoundary ? ( - - ) : 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: , + errorElement: hasErrorBoundary ? ( + + ) : 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, routeModulesCache: RouteModules, future: FutureConfig, - parentId?: string + parentId: string = "", + routesByParentId: Record< + string, + Omit[] + > = 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: , - errorElement: hasErrorBoundary ? ( - - ) : 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: , + errorElement: hasErrorBoundary ? ( + + ) : 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 index 6c63ae870b1..c90802e1cb7 100644 --- 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,16 +40,34 @@ export interface ServerRoute extends Route { module: ServerRouteModule; } +function groupRoutesByParentId(manifest: ServerRouteManifest) { + let routes: Record[]> = {}; + + 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[] + > = 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 @@ -56,54 +75,61 @@ export function createRoutes( export function createStaticHandlerDataRoutes( manifest: ServerRouteManifest, future: FutureConfig, - parentId?: string + parentId: string = "", + routesByParentId: Record< + string, + Omit[] + > = 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, + }; + }); }