test(app-router): harden RSC route matching delegation assertion by NathanDrake2406 · Pull Request #954 · cloudflare/vinext

NathanDrake2406 · 2026-04-29T03:46:12Z

What this changes

This updates the RSC route matching regression test now that the route/intercept matching runtime lives in createAppRscRouteMatcher.

Approach

Keep the generated-entry assertion focused on delegation to createAppRscRouteMatcher.
Check the extracted route-matching helper source for accidental decodeURIComponent(...) calls so the no-double-decode invariant follows the new ownership boundary.

Validation

vp check tests/shims.test.ts
vp test run tests/shims.test.ts -t "RSC route matching does not double-decode pathnames"
vp test run tests/shims.test.ts tests/app-rsc-route-matching.test.ts tests/entry-templates.test.ts

pkg-pr-new · 2026-04-29T03:46:53Z

Open in StackBlitz

npm i https://pkg.pr.new/vinext@954

commit: c9c39cd

james-elicx · 2026-04-29T06:36:33Z

/bigbonk review

ask-bonk

Clean extraction. The generated entry sheds ~170 lines of inline route-matching logic and delegates to two focused, testable modules. The split between build-time codegen (app-rsc-route-manifest.ts) and runtime matching (app-rsc-route-matching.ts) is well-drawn. Tests cover the key behaviors.

Two minor observations below — neither is blocking.

ask-bonk · 2026-04-29T06:38:36Z

+type RscRouteManifestCode = {
+  readonly routeEntries: readonly string[];
+  rootNotFoundVar: string | null;
+  rootForbiddenVar: string | null;


Nit: rootNotFoundVar, rootForbiddenVar, rootUnauthorizedVar, and globalErrorVar are mutable here but the sibling array properties (routeEntries, rootLayoutVars) are readonly. Since emitRscRouteManifest returns a fresh object and no caller mutates it, these could be readonly too for consistency.

Not blocking — just a consistency nit.

ask-bonk · 2026-04-29T06:38:36Z

+};
+
+type AppRscMatchableRoute<TPage = unknown, TLayout = unknown> = {
+  readonly patternParts: string[];


Worth noting for a future PR: the generated entry still carries its own copy of matchPattern for dynamic metadata route matching (line 819 of the generated output). Both copies are identical in logic. A follow-up could export matchPattern from this module and import it in the generated entry to eliminate the duplication. Not worth doing in this PR since metadata route handling is explicitly out of scope.

ask-bonk · 2026-04-29T06:38:47Z

Review submitted. Here's a summary of what I reviewed and the feedback:

Approved — this is a clean refactoring that follows the repo's guidance on keeping generated entries thin.

What the PR does well:

Extracts ~170 lines of inline route matching (matchRoute, findIntercept, interceptLookup construction, mergeMatchedParams) from the generated RSC entry into app-rsc-route-matching.ts — a typed, directly testable runtime helper
Separately extracts the import registry and route manifest codegen into app-rsc-route-manifest.ts — keeping build-time codegen concerns apart from runtime behavior
Both new modules have focused tests, and the existing entry-template snapshots and shims tests are updated to reflect the delegation

Two minor observations (non-blocking):

RscRouteManifestCode has inconsistent readonly modifiers — the array properties are readonly but the scalar string | null properties aren't. Since no caller mutates the returned object, they could all be readonly for consistency.
matchPattern is now duplicated — one copy lives in app-rsc-route-matching.ts (for intercepts) and an identical untyped copy remains in the generated entry (for dynamic metadata routes). A future PR could export it from the helper to eliminate the duplication, but that's correctly out of scope for this extraction.

github run

Copilot

Pull request overview

Updates App Router RSC route matching to be delegated to a shared runtime helper and hardens the associated regression tests around “no double decode” behavior.

Changes:

Add createAppRscRouteMatcher / matchAppRscRoutePattern runtime helper for RSC route + intercept matching.
Update generateRscEntry output to delegate route/intercept/metadata pattern matching to the helper (and refresh snapshots).
Strengthen tests to assert delegation and ensure the helper does not introduce decodeURIComponent(...) calls.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/shims.test.ts	Updates the “no double decode” assertion to check delegation + helper source.
tests/entry-templates.test.ts	Adds an assertion that generated RSC entry delegates matching to the shared helper.
tests/app-rsc-route-matching.test.ts	Adds unit tests for the new shared RSC route matching helper.
tests/snapshots/entry-templates.test.ts.snap	Updates snapshots for the new generated entry output.
packages/vinext/src/server/app-rsc-route-matching.ts	Introduces the shared runtime helper for RSC route + intercept matching.
packages/vinext/src/entries/app-rsc-entry.ts	Switches generated code to use the shared helper for matching.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-29T06:57:34Z

    // Verify it does NOT call decodeURIComponent (the comment mentions it but
    // should not have an actual call like `decodeURIComponent(...)`)


This test comment says the helper’s comment mentions decodeURIComponent, but app-rsc-route-matching.ts no longer contains that mention. Updating the comment to reflect what’s actually being asserted (no decodeURIComponent( call in the helper source) will avoid confusion for future maintainers.

Suggested change

// Verify it does NOT call decodeURIComponent (the comment mentions it but

// should not have an actual call like `decodeURIComponent(...)`)

// Verify the helper source does NOT contain a `decodeURIComponent(` call.

Copilot · 2026-04-29T06:57:34Z

+    if (patternPart.endsWith("+")) {
+      if (i !== patternParts.length - 1) return null;
+      const paramName = patternPart.slice(1, -1);
+      const remaining = urlParts.slice(i);
+      if (remaining.length === 0) return null;


The + catch-all branch triggers on any patternPart ending with +, even if it’s a static segment (e.g. a literal directory name like c++). This makes matchAppRscRoutePattern inconsistent with trie route parsing (see routing/route-trie.ts, which only treats + as special when the segment starts with :) and can cause incorrect intercept/metadata matching for routes whose static segments end in +. Restrict the +/* handling to patternPart.startsWith(":") (otherwise fall through to static equality).

Copilot · 2026-04-29T06:57:34Z

+    if (patternPart.endsWith("*")) {
+      if (i !== patternParts.length - 1) return null;
+      const paramName = patternPart.slice(1, -1);
+      params[paramName] = urlParts.slice(i);
+      return params;


Same as the + case above: the * branch treats any static segment ending in * as an optional catch-all. This can mis-match literal segments (e.g. star*) and diverges from the trie’s :name* convention. Guard this branch with patternPart.startsWith(":") so only :param* is treated as optional catch-all.

james-elicx · 2026-04-29T15:24:32Z

/bigbonk review

ask-bonk

Clean follow-up. The test correctly shifts from regex-extracting matchRoute out of a generated string to asserting delegation (createAppRscRouteMatcher import + __routeMatcher.matchRoute(url) call) and then checking the actual helper source for decodeURIComponent. One stale comment nit below — not blocking.

ask-bonk · 2026-04-29T15:26:26Z

+      "utf8",
+    );
    // Verify it does NOT call decodeURIComponent (the comment mentions it but
    // should not have an actual call like `decodeURIComponent(...)`)


Nit: this parenthetical is left over from when the generated code had an inline comment mentioning decodeURIComponent. The helper source (app-rsc-route-matching.ts) has no such comment — it just has a plain note about preserving normalized segment bytes. The assertion is still correct (checking for no decodeURIComponent( call), but the explanation is stale.

Suggested change

// should not have an actual call like `decodeURIComponent(...)`)

// The helper must NOT call decodeURIComponent — the request entry point

// owns decoding; matching preserves already-normalized segment bytes.

ask-bonk · 2026-04-29T15:26:41Z

Approved with one minor nit.

Summary: This is a small, focused follow-up to PR #953. The test previously regex-extracted matchRoute from the generated RSC entry string and checked it for decodeURIComponent calls. Now that #953 extracted route matching into app-rsc-route-matching.ts, this PR correctly updates the test to:

Assert the generated entry delegates to createAppRscRouteMatcher (import + call)
Read the actual helper source file and verify it contains no decodeURIComponent( calls

The one nit: the comment on line 3609–3610 ("the comment mentions it but should not have an actual call") is stale — the helper source has no such comment. Suggested a clearer replacement that explains the invariant directly.

github run

ask-bonk Bot approved these changes Apr 29, 2026

View reviewed changes

NathanDrake2406 force-pushed the nathan/rsc-route-matching-helper branch from cdcd9aa to c732b2b Compare April 29, 2026 06:50

NathanDrake2406 changed the title ~~refactor(app-router): delegate RSC route matching~~ test(app-router): harden RSC route matching delegation assertion Apr 29, 2026

NathanDrake2406 marked this pull request as ready for review April 29, 2026 06:52

Copilot AI review requested due to automatic review settings April 29, 2026 06:52

NathanDrake2406 marked this pull request as draft April 29, 2026 06:52

Copilot started reviewing on behalf of NathanDrake2406 April 29, 2026 06:52 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

test(app-router): update route matching delegation assertion

c9c39cd

NathanDrake2406 force-pushed the nathan/rsc-route-matching-helper branch from c732b2b to c9c39cd Compare April 29, 2026 15:21

NathanDrake2406 marked this pull request as ready for review April 29, 2026 15:21

ask-bonk Bot approved these changes Apr 29, 2026

View reviewed changes

james-elicx merged commit e3bc380 into cloudflare:main Apr 29, 2026
25 checks passed

NathanDrake2406 deleted the nathan/rsc-route-matching-helper branch May 6, 2026 04:30

		// Verify it does NOT call decodeURIComponent (the comment mentions it but
		// should not have an actual call like `decodeURIComponent(...)`)

	// Verify it does NOT call decodeURIComponent (the comment mentions it but
	// should not have an actual call like `decodeURIComponent(...)`)
	// Verify the helper source does NOT contain a `decodeURIComponent(` call.

	// should not have an actual call like `decodeURIComponent(...)`)
	// The helper must NOT call decodeURIComponent — the request entry point
	// owns decoding; matching preserves already-normalized segment bytes.

Conversation

NathanDrake2406 commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this changes

Approach

Validation

Uh oh!

pkg-pr-new Bot commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

james-elicx commented Apr 29, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented Apr 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

james-elicx commented Apr 29, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented Apr 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NathanDrake2406 commented Apr 29, 2026 •

edited

Loading

pkg-pr-new Bot commented Apr 29, 2026 •

edited

Loading