Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Feb 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/many-pans-attack.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@remix-run/react": patch
"@remix-run/server-runtime": patch
---

Improve efficiency of route manifest->tree transformation
160 changes: 91 additions & 69 deletions packages/remix-react/routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
136 changes: 81 additions & 55 deletions packages/remix-server-runtime/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
};
});
}