fix(app-router): tag cached pages by route pattern by NathanDrake2406 · Pull Request #923 · cloudflare/vinext

NathanDrake2406 · 2026-04-28T00:41:16Z

What this changes

App Router page and route handler cache entries now carry implicit tags derived from the matched route pattern in addition to exact resolved pathname tags.

Why

revalidatePath(path, type) targets _N_T_ tags based on route files. Entries rendered at dynamic URLs need tags such as _N_T_/blog/[slug]/layout so layout-scoped invalidation reaches cached fetches beneath dynamic routes.

Refs #921

Approach

Move implicit tag construction into server/implicit-tags.ts and wire generated RSC entries to pass route.routeSegments plus the leaf kind. The helper preserves exact pathname tags, root/index aliases, route groups, dynamic segments, page leaves, and route handler leaves.

This is scoped to App Router cache tag generation and does not change ISR storage semantics.

Validation

vp test run tests/page-cache-tags.test.ts tests/isr-cache.test.ts tests/fetch-cache.test.ts -t "App Router page cache tags|revalidatePath|soft tags"
vp test run tests/app-router.test.ts -t "generated code threads collected fetch tags into page ISR writes|generated code stores rscData in the ISR cache entry"
vp test run tests/entry-templates.test.ts
vp check
vp run vinext#build

pkg-pr-new · 2026-04-28T00:42:01Z

Open in StackBlitz

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

commit: 3ad9bad

Copilot

Pull request overview

Updates vinext’s App Router cache tag generation so cached fetch/page/route-handler entries are tagged using the matched route pattern (including layout chain) in addition to exact resolved pathname tags, enabling revalidatePath() to propagate through nested/dynamic routes as intended.

Changes:

Introduces buildPageCacheTags() helper to centralize implicit tag construction (pattern-derived layout/page/route leaves + exact pathname tags).
Wires generated App Router RSC entry code to pass route.routeSegments and leaf kind into buildPageCacheTags() for soft tags and ISR writes.
Adds/updates tests and snapshots to validate the new implicit tag behavior; adjusts knip config for the local-only vitest reporter dependency.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/page-cache-tags.test.ts	Adds focused unit coverage for pattern-derived implicit tags (dynamic segments, route groups, route handlers).
tests/isr-cache.test.ts	Replaces local implicit-tag derivation with `buildPageCacheTags()` in revalidatePath-related tests.
tests/app-router.test.ts	Updates codegen assertions to expect `buildPageCacheTags()` usage in generated RSC entry code.
tests/snapshots/entry-templates.test.ts.snap	Updates snapshots to reflect removal of inline `__pageCacheTags` and new import/usage of `buildPageCacheTags()`.
packages/vinext/src/server/implicit-tags.ts	Adds centralized implicit tag generation helper used by App Router caching.
packages/vinext/src/entries/app-rsc-entry.ts	Imports and uses `buildPageCacheTags()` when setting soft tags and when writing ISR entries.
knip.ts	Makes the `agent` dependency ignored only outside CI.

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

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

NathanDrake2406 · 2026-04-28T08:26:47Z

/bigbonk review this PR. Focus on blocking correctness issues and test gaps.

NathanDrake2406 · 2026-04-28T08:47:43Z

/bigbonk review this PR. Focus on blocking correctness issues and whether the tests cover the real behavior and meaningful edge cases.

ask-bonk · 2026-04-28T08:48:23Z

User NathanDrake2406 does not have write permissions

github run

ask-bonk · 2026-04-28T08:48:52Z

@NathanDrake2406 Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

ask-bonk · 2026-04-28T08:49:01Z

@NathanDrake2406 Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

ask-bonk · 2026-04-28T08:49:19Z

@james-elicx Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

james-elicx · 2026-04-28T09:06:57Z

/bigbonk review

ask-bonk · 2026-04-28T09:07:36Z

UnknownError: ProviderInitError

github run

james-elicx · 2026-04-28T09:21:31Z

/bigbonk review

ask-bonk

Good extraction. Moving __pageCacheTags out of the generated entry template into a typed, testable module is the right direction per the repo guidelines on keeping entries thin. The core tag generation logic is correct and the test coverage for the helper itself is solid.

I have two concerns — one blocking, one non-blocking — detailed in inline comments below.

ask-bonk · 2026-04-28T09:27:04Z

Review posted. Here's a summary of what I flagged:

Blocking issue: The ISR cache tests (tests/isr-cache.test.ts) only use static routes via the staticRouteSegments helper, so they don't cover the key semantic change this PR introduces — dynamic routes where routeSegments (pattern-based) and pathname (resolved) diverge. A test should verify that revalidatePath("/blog/[slug]", "layout") works for entries cached at /blog/hello, and that the typeless revalidatePath("/blog/hello") still works via the exact pathname tag.

Non-blocking: The :param → [param] conversion in normalizeRouteSegment is dead code since routeSegments already uses bracket notation from the filesystem. The "." and "@" guards are similarly defensive. Not wrong, but worth a clarifying comment.

github run

james-elicx · 2026-04-28T11:23:03Z

/bigbonk review

ask-bonk

Review

Good extraction. Moving the inline __pageCacheTags out of the generated entry template into a testable implicit-tags.ts module is the right call — it follows the repo guideline of keeping entries thin and pushing real behavior into typed helpers.

The core tag generation logic is correct. I traced through appendDerivedTags for several inputs (dynamic segments, root pages, route handlers, literal page/route path segments) and the leaf detection via position-based indexing is sound. The ISR integration test now covers the key dynamic-route semantic change (pattern-based typed invalidation vs exact pathname no-type invalidation).

Two non-blocking issues below.

ask-bonk · 2026-04-28T11:26:17Z

Review posted. Here's a summary of what I flagged:

No blocking issues. The core logic is correct — I traced appendDerivedTags for dynamic segments, root pages, route handlers, and literal page/route path segments. The position-based leaf detection is sound and the ISR integration test covers the key semantic change (pattern-based typed invalidation vs exact pathname no-type invalidation).

Two non-blocking suggestions:

Loop readability (implicit-tags.ts:33): i < routeParts.length + 1 could be i <= routeParts.length for clarity.
Missing catch-all test coverage (page-cache-tags.test.ts): No test for [...slug] or [[...id]] segments. They should pass through normalizeRouteSegment unchanged, but an explicit test would document the behavior and prevent regressions.

I also noticed a stale comment in packages/vinext/src/shims/cache.ts:382 still referencing __pageCacheTags (now buildPageCacheTags), but that file isn't part of this PR so I left it out of the inline review.

github run

Cached App Router pages only carried implicit revalidatePath tags derived from the resolved URL. That left dynamic routes tagged with concrete values, so pattern-scoped invalidation like revalidatePath("/blog/[slug]", "layout") could miss entries rendered at /blog/hello. The generated RSC entry now delegates implicit tag construction to a typed helper that combines exact pathname tags with route-segment-derived layout, page, and route handler tags. Focused tests cover dynamic route patterns, route groups, root aliases, and route handler leaves. Refs cloudflare#921

james-elicx · 2026-04-28T16:44:16Z

/bigbonk review

ask-bonk

Good change. Extracting __pageCacheTags from the generated entry template into a typed, testable implicit-tags.ts module is the right direction per repo guidelines. The core logic is correct — I traced appendDerivedTags through dynamic segments, root pages, route groups, catch-alls, and route handlers. Position-based leaf detection is sound and avoids the original Copilot-flagged issue with literal page/route path segments.

CI is all green. Test coverage is solid: the new page-cache-tags.test.ts covers the helper thoroughly, and the ISR integration test now covers the key semantic change (pattern-based typed invalidation vs exact pathname no-type invalidation for dynamic routes).

Two minor non-blocking observations inline.

ask-bonk · 2026-04-28T16:46:30Z

+
+    const isLeaf = i - 1 === leafIndex;
+    if (!isLeaf) {
+      currentPathname = `${currentPathname}${currentPathname.endsWith("/") ? "" : "/"}layout`;


Non-blocking: The currentPathname.endsWith("/") guard is unreachable. Since routePath always starts with / (checked at line 29) and segments come from splitting by /, routeParts.slice(0, i).join("/") for i >= 2 will always produce paths like /blog or /blog/[slug] — never ending in /. The guard is harmless but could be replaced with a comment noting it's defensive, or just dropped.

hmm, yeah, reading the logic, i dont think it's possible to end with / inside this loop because of the leaf check

ask-bonk · 2026-04-28T16:46:30Z

        basePath: __basePath,
-        buildPageCacheTags: __pageCacheTags,
+        buildPageCacheTags: function(pathname, extraTags) {


Non-blocking (readability): This closure-wrapping pattern appears identically in two places (the cached route handler path and the uncached route handler path). Consider extracting a named helper inside _handleRequest to avoid the duplication, e.g.:

Suggested change

basePath: __basePath,

buildPageCacheTags: __pageCacheTags,

buildPageCacheTags: function(pathname, extraTags) {

buildPageCacheTags: (pathname, extraTags) =>

buildPageCacheTags(pathname, extraTags, route.routeSegments, "route"),

Also, arrow function form is shorter and equivalent here since this isn't used. Same applies to the other identical instance at line ~2211.

ask-bonk · 2026-04-28T16:46:39Z

Review submitted. Here's a summary:

Approved with two non-blocking observations:

implicit-tags.ts:39 — The currentPathname.endsWith("/") guard is unreachable since split-by-/ segments joined back together will never produce a trailing slash for non-leaf entries. Harmless but could be dropped or annotated.
app-rsc-entry.ts:2163-2165 — The function(pathname, extraTags) { return buildPageCacheTags(...) } closure-wrapping pattern appears identically in two places. Could be an arrow function (shorter, equivalent since this isn't used) or extracted to a local helper to reduce duplication.

The core logic is correct — I traced appendDerivedTags through all the key cases (dynamic segments, root pages, route groups, catch-alls, route handlers, literal page/route path segments). The position-based leaf detection is sound. CI is fully green. Test coverage addresses the previous reviewer's blocking concern about dynamic route semantic changes.

github run

NathanDrake2406 force-pushed the nathan/revalidate-path-pattern-tags branch from cf005ab to 8fd9314 Compare April 28, 2026 00:45

NathanDrake2406 marked this pull request as ready for review April 28, 2026 00:49

Copilot AI review requested due to automatic review settings April 28, 2026 00:49

Copilot started reviewing on behalf of NathanDrake2406 April 28, 2026 00:50 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

Comment thread packages/vinext/src/server/implicit-tags.ts Outdated

NathanDrake2406 force-pushed the nathan/revalidate-path-pattern-tags branch from 8fd9314 to 9efe607 Compare April 28, 2026 01:03

NathanDrake2406 requested a review from Copilot April 28, 2026 01:05

Copilot started reviewing on behalf of NathanDrake2406 April 28, 2026 01:45 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

ask-bonk Bot reviewed Apr 28, 2026

View reviewed changes

Comment thread packages/vinext/src/server/implicit-tags.ts Outdated

Comment thread tests/isr-cache.test.ts Outdated

NathanDrake2406 force-pushed the nathan/revalidate-path-pattern-tags branch from 9efe607 to 576d89d Compare April 28, 2026 09:30

ask-bonk Bot reviewed Apr 28, 2026

View reviewed changes

Comment thread packages/vinext/src/server/implicit-tags.ts Outdated

Comment thread tests/page-cache-tags.test.ts

NathanDrake2406 force-pushed the nathan/revalidate-path-pattern-tags branch from 576d89d to 3ad9bad Compare April 28, 2026 15:08

ask-bonk Bot approved these changes Apr 28, 2026

View reviewed changes

Conversation

NathanDrake2406 commented Apr 28, 2026

What this changes

Why

Approach

Validation

Uh oh!

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

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

NathanDrake2406 commented Apr 28, 2026

Uh oh!

NathanDrake2406 commented Apr 28, 2026

Uh oh!

ask-bonk Bot commented Apr 28, 2026

Uh oh!

ask-bonk Bot commented Apr 28, 2026

Uh oh!

ask-bonk Bot commented Apr 28, 2026

Uh oh!

ask-bonk Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

james-elicx commented Apr 28, 2026

Uh oh!

ask-bonk Bot commented Apr 28, 2026

Uh oh!

james-elicx commented Apr 28, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ask-bonk Bot commented Apr 28, 2026

Uh oh!

james-elicx commented Apr 28, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Review

Uh oh!

Uh oh!

Uh oh!

ask-bonk Bot commented Apr 28, 2026

Uh oh!

james-elicx commented Apr 28, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

james-elicx Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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

edited

Loading

ask-bonk Bot commented Apr 28, 2026 •

edited

Loading