Skip to content

Commit 33dfe26

Browse files
perf: optimize resolveParentParams lookups (#718)
* test: add direct unit tests for resolveParentParams * perf: optimize resolveParentParams lookups - Use Map<pattern, route> index for O(1) route lookups (was O(N) linear scan) - Use pre-computed patternParts instead of re-splitting pattern string - Pre-compute last dynamic segment index (was per-iteration slice+some) * test: add edge case tests for resolveParentParams Cover fully static routes, single-segment dynamic routes, and catch-all child segments. * refactor: simplify resolveParentParams internals - Remove dead ParentSegment.params field (allocated but never read) - Replace slice+join with incremental prefix string building - Export StaticParamsMap type so tests derive from source - Improve comment to explain WHY last segment is excluded * refactor: address bonk review nits in resolveParentParams - Move StaticParamsMap type above JSDoc block so the function comment sits directly above resolveParentParams (was erroneously separating them) - Declare 'part' before building prefixPattern so it's used for both the prefix string concatenation and the startsWith check, avoiding redundant array indexing --------- Co-authored-by: James <james@eli.cx>
1 parent a7dd2d5 commit 33dfe26

File tree

2 files changed

+182
-41
lines changed

2 files changed

+182
-41
lines changed

packages/vinext/src/build/prerender.ts

Lines changed: 40 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -245,45 +245,54 @@ export function getOutputPath(urlPath: string, trailingSlash: boolean): string {
245245
return `${clean}.html`;
246246
}
247247

248+
/** Map of route patterns to generateStaticParams functions (or null/undefined). */
249+
export type StaticParamsMap = Record<
250+
string,
251+
| ((opts: {
252+
params: Record<string, string | string[]>;
253+
}) => Promise<Record<string, string | string[]>[]>)
254+
| null
255+
| undefined
256+
>;
257+
248258
/**
249259
* Resolve parent dynamic segment params for a route.
250260
* Handles top-down generateStaticParams resolution for nested dynamic routes.
251261
*
252262
* Uses the `staticParamsMap` (pattern → generateStaticParams) exported from
253263
* the production bundle.
254264
*/
255-
async function resolveParentParams(
265+
export async function resolveParentParams(
256266
childRoute: AppRoute,
257-
allRoutes: AppRoute[],
258-
staticParamsMap: Record<
259-
string,
260-
| ((opts: {
261-
params: Record<string, string | string[]>;
262-
}) => Promise<Record<string, string | string[]>[]>)
263-
| null
264-
| undefined
265-
>,
267+
routeIndex: ReadonlyMap<string, AppRoute>,
268+
staticParamsMap: StaticParamsMap,
266269
): Promise<Record<string, string | string[]>[]> {
267-
const patternParts = childRoute.pattern.split("/").filter(Boolean);
270+
const { patternParts } = childRoute;
271+
272+
// The last dynamic segment belongs to the child route itself — its params
273+
// are resolved by the child's own generateStaticParams. We only collect
274+
// params from earlier (parent) dynamic segments.
275+
let lastDynamicIdx = -1;
276+
for (let i = patternParts.length - 1; i >= 0; i--) {
277+
if (patternParts[i].startsWith(":")) {
278+
lastDynamicIdx = i;
279+
break;
280+
}
281+
}
268282

269-
type ParentSegment = {
270-
params: string[];
271-
generateStaticParams: (opts: {
272-
params: Record<string, string | string[]>;
273-
}) => Promise<Record<string, string | string[]>[]>;
274-
};
283+
type GenerateStaticParamsFn = (opts: {
284+
params: Record<string, string | string[]>;
285+
}) => Promise<Record<string, string | string[]>[]>;
275286

276-
const parentSegments: ParentSegment[] = [];
287+
const parentSegments: GenerateStaticParamsFn[] = [];
277288

278-
for (let i = 0; i < patternParts.length; i++) {
289+
let prefixPattern = "";
290+
for (let i = 0; i < lastDynamicIdx; i++) {
279291
const part = patternParts[i];
292+
prefixPattern += "/" + part;
280293
if (!part.startsWith(":")) continue;
281294

282-
const isLastDynamicPart = !patternParts.slice(i + 1).some((p) => p.startsWith(":"));
283-
if (isLastDynamicPart) break;
284-
285-
const prefixPattern = "/" + patternParts.slice(0, i + 1).join("/");
286-
const parentRoute = allRoutes.find((r) => r.pattern === prefixPattern);
295+
const parentRoute = routeIndex.get(prefixPattern);
287296
// TODO: layout-level generateStaticParams — a layout segment can define
288297
// generateStaticParams without a corresponding page file, so parentRoute
289298
// may be undefined here even though the layout exports generateStaticParams.
@@ -293,22 +302,18 @@ async function resolveParentParams(
293302
if (parentRoute?.pagePath) {
294303
const fn = staticParamsMap[prefixPattern];
295304
if (typeof fn === "function") {
296-
const paramName = part.replace(/^:/, "").replace(/[+*]$/, "");
297-
parentSegments.push({
298-
params: [paramName],
299-
generateStaticParams: fn,
300-
});
305+
parentSegments.push(fn);
301306
}
302307
}
303308
}
304309

305310
if (parentSegments.length === 0) return [];
306311

307312
let currentParams: Record<string, string | string[]>[] = [{}];
308-
for (const segment of parentSegments) {
313+
for (const generateStaticParams of parentSegments) {
309314
const nextParams: Record<string, string | string[]>[] = [];
310315
for (const parentParams of currentParams) {
311-
const results = await segment.generateStaticParams({ params: parentParams });
316+
const results = await generateStaticParams({ params: parentParams });
312317
if (Array.isArray(results)) {
313318
for (const result of results) {
314319
nextParams.push({ ...parentParams, ...result });
@@ -690,14 +695,7 @@ export async function prerenderApp({
690695
const serverDir = path.dirname(rscBundlePath);
691696

692697
let rscHandler: (request: Request) => Promise<Response>;
693-
let staticParamsMap: Record<
694-
string,
695-
| ((opts: {
696-
params: Record<string, string | string[]>;
697-
}) => Promise<Record<string, string | string[]>[]>)
698-
| null
699-
| undefined
700-
> = {};
698+
let staticParamsMap: StaticParamsMap = {};
701699
// ownedProdServer: a prod server we started ourselves and must close in finally.
702700
// When the caller passes options._prodServer we use that and do NOT close it.
703701
let ownedProdServerHandle: { server: HttpServer; port: number } | null = null;
@@ -804,6 +802,8 @@ export async function prerenderApp({
804802
},
805803
});
806804

805+
const routeIndex = new Map(routes.map((r) => [r.pattern, r]));
806+
807807
// ── Collect URLs to render ────────────────────────────────────────────────
808808
type UrlToRender = {
809809
urlPath: string;
@@ -887,7 +887,7 @@ export async function prerenderApp({
887887
continue;
888888
}
889889

890-
const parentParamSets = await resolveParentParams(route, routes, staticParamsMap);
890+
const parentParamSets = await resolveParentParams(route, routeIndex, staticParamsMap);
891891
let paramSets: Record<string, string | string[]>[] | null;
892892

893893
if (parentParamSets.length > 0) {

tests/prerender.test.ts

Lines changed: 142 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,12 @@ import fs from "node:fs";
1313
import os from "node:os";
1414
import path from "node:path";
1515
import { buildPagesFixture, buildAppFixture, buildCloudflareAppFixture } from "./helpers.js";
16-
import type { PrerenderRouteResult } from "../packages/vinext/src/build/prerender.js";
16+
import {
17+
resolveParentParams,
18+
type PrerenderRouteResult,
19+
type StaticParamsMap,
20+
} from "../packages/vinext/src/build/prerender.js";
21+
import type { AppRoute } from "../packages/vinext/src/routing/app-router.js";
1722

1823
const PAGES_FIXTURE = path.resolve(import.meta.dirname, "./fixtures/pages-basic");
1924
const APP_FIXTURE = path.resolve(import.meta.dirname, "./fixtures/app-basic");
@@ -719,3 +724,139 @@ describe("Cloudflare Workers hybrid build (cf-app-basic)", () => {
719724
});
720725
});
721726
});
727+
728+
// ─── resolveParentParams unit tests ─────────────────────────────────────────
729+
730+
function mockRoute(pattern: string, opts: { pagePath?: string | null } = {}): AppRoute {
731+
const parts = pattern.split("/").filter(Boolean);
732+
return {
733+
pattern,
734+
pagePath: opts.pagePath ?? `/app${pattern}/page.tsx`,
735+
routePath: null,
736+
layouts: [],
737+
templates: [],
738+
parallelSlots: [],
739+
loadingPath: null,
740+
errorPath: null,
741+
layoutErrorPaths: [],
742+
notFoundPath: null,
743+
notFoundPaths: [],
744+
forbiddenPath: null,
745+
unauthorizedPath: null,
746+
routeSegments: [],
747+
layoutTreePositions: [],
748+
isDynamic: parts.some((p) => p.startsWith(":")),
749+
params: parts
750+
.filter((p) => p.startsWith(":"))
751+
.map((p) => p.replace(/^:/, "").replace(/[+*]$/, "")),
752+
patternParts: parts,
753+
};
754+
}
755+
756+
function routeIndexFrom(routes: AppRoute[]): ReadonlyMap<string, AppRoute> {
757+
return new Map(routes.map((r) => [r.pattern, r]));
758+
}
759+
760+
describe("resolveParentParams", () => {
761+
it("returns empty array when route has no parent dynamic segments", async () => {
762+
const route = mockRoute("/blog/:slug");
763+
const result = await resolveParentParams(route, routeIndexFrom([route]), {});
764+
expect(result).toEqual([]);
765+
});
766+
767+
it("returns empty array when parent route has no pagePath", async () => {
768+
const parent = mockRoute("/shop/:category", { pagePath: null });
769+
const child = mockRoute("/shop/:category/:item");
770+
const result = await resolveParentParams(child, routeIndexFrom([parent, child]), {});
771+
expect(result).toEqual([]);
772+
});
773+
774+
it("returns empty array when parent has no generateStaticParams", async () => {
775+
const parent = mockRoute("/shop/:category");
776+
const child = mockRoute("/shop/:category/:item");
777+
const staticParamsMap: StaticParamsMap = {};
778+
const result = await resolveParentParams(
779+
child,
780+
routeIndexFrom([parent, child]),
781+
staticParamsMap,
782+
);
783+
expect(result).toEqual([]);
784+
});
785+
786+
it("resolves single parent dynamic segment", async () => {
787+
const parent = mockRoute("/shop/:category");
788+
const child = mockRoute("/shop/:category/:item");
789+
const staticParamsMap: StaticParamsMap = {
790+
"/shop/:category": async () => [{ category: "electronics" }, { category: "clothing" }],
791+
};
792+
const result = await resolveParentParams(
793+
child,
794+
routeIndexFrom([parent, child]),
795+
staticParamsMap,
796+
);
797+
expect(result).toEqual([{ category: "electronics" }, { category: "clothing" }]);
798+
});
799+
800+
it("resolves two levels of parent dynamic segments", async () => {
801+
const grandparent = mockRoute("/a/:b");
802+
const parent = mockRoute("/a/:b/c/:d");
803+
const child = mockRoute("/a/:b/c/:d/:e");
804+
const staticParamsMap: StaticParamsMap = {
805+
"/a/:b": async () => [{ b: "1" }, { b: "2" }],
806+
"/a/:b/c/:d": async ({ params }) => {
807+
if (params.b === "1") return [{ d: "x" }];
808+
return [{ d: "y" }, { d: "z" }];
809+
},
810+
};
811+
const result = await resolveParentParams(
812+
child,
813+
routeIndexFrom([grandparent, parent, child]),
814+
staticParamsMap,
815+
);
816+
expect(result).toEqual([
817+
{ b: "1", d: "x" },
818+
{ b: "2", d: "y" },
819+
{ b: "2", d: "z" },
820+
]);
821+
});
822+
823+
it("skips static segments between dynamic parents", async () => {
824+
const parent = mockRoute("/shop/:category");
825+
const child = mockRoute("/shop/:category/details/:item");
826+
const staticParamsMap: StaticParamsMap = {
827+
"/shop/:category": async () => [{ category: "shoes" }],
828+
};
829+
const result = await resolveParentParams(
830+
child,
831+
routeIndexFrom([parent, child]),
832+
staticParamsMap,
833+
);
834+
expect(result).toEqual([{ category: "shoes" }]);
835+
});
836+
837+
it("returns empty array for a fully static route", async () => {
838+
const route = mockRoute("/about/contact");
839+
const result = await resolveParentParams(route, routeIndexFrom([route]), {});
840+
expect(result).toEqual([]);
841+
});
842+
843+
it("returns empty array for a single-segment dynamic route", async () => {
844+
const route = mockRoute("/:id");
845+
const result = await resolveParentParams(route, routeIndexFrom([route]), {});
846+
expect(result).toEqual([]);
847+
});
848+
849+
it("resolves parent with catch-all child segment", async () => {
850+
const parent = mockRoute("/shop/:category");
851+
const child = mockRoute("/shop/:category/:rest+");
852+
const staticParamsMap: StaticParamsMap = {
853+
"/shop/:category": async () => [{ category: "electronics" }],
854+
};
855+
const result = await resolveParentParams(
856+
child,
857+
routeIndexFrom([parent, child]),
858+
staticParamsMap,
859+
);
860+
expect(result).toEqual([{ category: "electronics" }]);
861+
});
862+
});

0 commit comments

Comments
 (0)