Skip to content
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
17 changes: 11 additions & 6 deletions packages/vinext/src/routing/route-trie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ export function trieMatch<R>(
return match(root, urlParts, 0);
}

function createParams(): Record<string, string | string[]> {
return Object.create(null);
}

function match<R>(
node: TrieNode<R>,
urlParts: string[],
Expand All @@ -142,14 +146,15 @@ function match<R>(
if (index === urlParts.length) {
// Exact match at this node
if (node.route !== null) {
return { route: node.route, params: Object.create(null) };
return { route: node.route, params: createParams() };
}

// Optional catch-all with 0 segments
if (node.optionalCatchAllChild !== null) {
const params: Record<string, string | string[]> = Object.create(null);
params[node.optionalCatchAllChild.paramName] = [];
return { route: node.optionalCatchAllChild.route, params };
return {
route: node.optionalCatchAllChild.route,
params: createParams(),
};
}

return null;
Expand Down Expand Up @@ -178,15 +183,15 @@ function match<R>(
// 3. Try catch-all (1+ remaining segments)
if (node.catchAllChild !== null) {
const remaining = urlParts.slice(index);
const params: Record<string, string | string[]> = Object.create(null);
const params = createParams();
params[node.catchAllChild.paramName] = remaining;
return { route: node.catchAllChild.route, params };
}

// 4. Try optional catch-all (0+ remaining segments)
if (node.optionalCatchAllChild !== null) {
const remaining = urlParts.slice(index);
const params: Record<string, string | string[]> = Object.create(null);
const params = createParams();
params[node.optionalCatchAllChild.paramName] = remaining;
return { route: node.optionalCatchAllChild.route, params };
}
Expand Down
15 changes: 11 additions & 4 deletions packages/vinext/src/server/app-rsc-route-matching.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ type AppRscInterceptLookupEntry = {
params: readonly string[];
};

function createRouteParams(): AppRscRouteParams {
return Object.create(null);
}

export function createAppRscRouteMatcher<Route extends AppRscRouteForMatching>(
routes: Route[],
): {
Expand All @@ -55,7 +59,7 @@ export function createAppRscRouteMatcher<Route extends AppRscRouteForMatching>(
for (const entry of interceptLookup) {
const params = matchAppRscRoutePattern(urlParts, entry.targetPatternParts);
if (params !== null) {
let sourceParams: AppRscRouteParams = Object.create(null);
let sourceParams = createRouteParams();
if (sourcePathname !== null) {
const sourceRoute = routes[entry.sourceRouteIndex];
const sourceParts = sourcePathname.split("/").filter(Boolean);
Expand Down Expand Up @@ -103,7 +107,7 @@ export function matchAppRscRoutePattern(
urlParts: string[],
patternParts: string[],
): AppRscRouteParams | null {
const params: AppRscRouteParams = Object.create(null);
const params = createRouteParams();
for (let i = 0; i < patternParts.length; i++) {
const patternPart = patternParts[i];
if (patternPart.startsWith(":") && patternPart.endsWith("+")) {
Expand All @@ -117,7 +121,10 @@ export function matchAppRscRoutePattern(
if (patternPart.startsWith(":") && patternPart.endsWith("*")) {
if (i !== patternParts.length - 1) return null;
const paramName = patternPart.slice(1, -1);
params[paramName] = urlParts.slice(i);
const remaining = urlParts.slice(i);
if (remaining.length > 0) {
params[paramName] = remaining;
}
return params;
}
if (patternPart.startsWith(":")) {
Expand All @@ -135,5 +142,5 @@ function mergeMatchedParams(
sourceParams: AppRscRouteParams,
targetParams: AppRscRouteParams,
): AppRscRouteParams {
return Object.assign(Object.create(null), sourceParams, targetParams);
return Object.assign(createRouteParams(), sourceParams, targetParams);
}
27 changes: 24 additions & 3 deletions tests/app-rsc-route-matching.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,30 @@ describe("App RSC route matching", () => {
route: { pattern: "/docs/:path+" },
params: { path: ["guides", "rsc"] },
});
expect(matcher.matchRoute("/shop")).toMatchObject({
route: { pattern: "/shop/:path*" },
params: { path: [] },
const result = matcher.matchRoute("/shop");
expect(result).not.toBeNull();
expect(result!.route.pattern).toBe("/shop/:path*");
expect(result!.params).toEqual({});
});

it("omits optional catch-all params when zero segments are matched", () => {
// Next.js represents a missing optional catch-all param as absent at the
// route-match boundary; app rendering later treats that as the `null`
// tree segment for optional catch-all.
// https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/router/utils/route-matcher.ts
// https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/router/utils/get-dynamic-param.test.ts
const matcher = createAppRscRouteMatcher([route("/shop/:path*", ["shop", ":path*"])]);

const result = matcher.matchRoute("/shop");
expect(result).not.toBeNull();
expect(result!.route.pattern).toBe("/shop/:path*");
expect(result!.params).toEqual({});
});

it("omits optional catch-all params from standalone route pattern matches", () => {
expect(matchAppRscRoutePattern(["shop"], ["shop", ":path*"])).toEqual({});
expect(matchAppRscRoutePattern(["shop", "a", "b"], ["shop", ":path*"])).toEqual({
path: ["a", "b"],
});
});

Expand Down
23 changes: 20 additions & 3 deletions tests/route-trie.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,17 @@ describe("buildRouteTrie + trieMatch", () => {
});

describe("optional catch-all routes", () => {
it("matches optional catch-all with zero segments", () => {
it("matches optional catch-all with zero segments without materializing the param", () => {
// Next.js route-regex makes the optional catch-all capture group optional, and
// route-matcher only writes params for defined capture groups:
// https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/router/utils/route-regex.ts
// https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/router/utils/route-matcher.ts
const routes = [r("/docs/:path*")];
const trie = buildRouteTrie(routes);

const result = trieMatch(trie, ["docs"]);
expect(result).not.toBeNull();
expect(result!.params).toEqual({ path: [] });
expect(result!.params).toEqual({});
});

it("matches optional catch-all with one segment", () => {
Expand All @@ -141,6 +145,16 @@ describe("buildRouteTrie + trieMatch", () => {
expect(result).not.toBeNull();
expect(result!.params).toEqual({ path: ["api", "ref"] });
});

it("matches root-level optional catch-all with zero segments without materializing the param", () => {
const routes = [r("/:path*")];
const trie = buildRouteTrie(routes);

const result = trieMatch(trie, []);
expect(result).not.toBeNull();
expect(result!.route.pattern).toBe("/:path*");
expect(result!.params).toEqual({});
});
});

describe("priority / precedence", () => {
Expand Down Expand Up @@ -280,7 +294,10 @@ describe("buildRouteTrie + trieMatch", () => {
if (pp.endsWith("*")) {
if (i !== patternParts.length - 1) return null;
const paramName = pp.slice(1, -1);
params[paramName] = urlParts.slice(i);
const remaining = urlParts.slice(i);
if (remaining.length > 0) {
params[paramName] = remaining;
}
return params;
Comment on lines 294 to 301
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this linearMatchPattern helper is the reference implementation that the parity fuzz test (below) compares the trie against. Updating it in the same PR as the trie change is correct — the two must stay in sync — but it's worth a one-line comment here noting why, so a future reader doesn't wonder if the test is just tautologically matching the implementation.

Something like:

Suggested change
if (pp.endsWith("*")) {
if (i !== patternParts.length - 1) return null;
const paramName = pp.slice(1, -1);
params[paramName] = urlParts.slice(i);
const remaining = urlParts.slice(i);
if (remaining.length > 0) {
params[paramName] = remaining;
}
return params;
if (pp.endsWith("*")) {
if (i !== patternParts.length - 1) return null;
const paramName = pp.slice(1, -1);
const remaining = urlParts.slice(i);
// Match Next.js: omit key when zero segments captured (parity with trie)
if (remaining.length > 0) {
params[paramName] = remaining;
}
return params;
}

}
if (pp.startsWith(":")) {
Expand Down
6 changes: 3 additions & 3 deletions tests/routing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -984,7 +984,7 @@ describe("matchAppRoute - URL matching", () => {
const result = matchAppRoute("/optional", routes);
expect(result).not.toBeNull();
expect(result!.route.pattern).toBe("/optional/:path*");
expect(result!.params.path).toEqual([]);
expect(result!.params).not.toHaveProperty("path");
});

it("matches optional catch-all with multiple segments", async () => {
Expand Down Expand Up @@ -1518,7 +1518,7 @@ describe("matchAppRoute - URL matching", () => {
const result = matchAppRoute("/sign-in", routes);
expect(result).not.toBeNull();
expect(result!.route.pattern).toBe("/sign-in/:sign-in*");
expect(result!.params["sign-in"]).toEqual([]);
expect(result!.params).not.toHaveProperty("sign-in");
});

it("matches hyphenated optional catch-all with segments", async () => {
Expand Down Expand Up @@ -1666,7 +1666,7 @@ describe("pagesRouter - hyphenated param names", () => {
const result = matchRoute("/sign-up", routes);
expect(result).not.toBeNull();
expect(result!.route.pattern).toBe("/sign-up/:sign-up*");
expect(result!.params["sign-up"]).toEqual([]);
expect(result!.params).not.toHaveProperty("sign-up");
});

it("matches hyphenated optional catch-all with segments", async () => {
Expand Down
Loading