fix(app-router): Make revalidatePath expire route-scoped fetch cache reads#917
Conversation
commit: |
There was a problem hiding this comment.
Pull request overview
This PR aligns vinext’s App Router fetch cache behavior with Next.js by threading route-derived implicit tags into fetch cache reads as soft tags, allowing revalidatePath() to force cached fetch() results to miss for the affected route without permanently coupling shared fetch entries to a specific path.
Changes:
- Add request-scoped
currentFetchSoftTagsstate and exposesetCurrentFetchSoftTags()for App Router render scopes. - Pass
softTagsinto fetch cache reads and treat revalidated soft tags as read-time misses in Memory/KV cache handlers (without deleting shared fetch entries). - Extend test coverage to validate soft-tag invalidation behavior (including KV handler behavior) and update generated entry template snapshots.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/kv-cache-handler.test.ts | Adds coverage ensuring softTags invalidate FETCH reads without deleting the shared KV entry. |
| tests/fetch-cache.test.ts | Adds coverage that revalidatePath() invalidates fetch cache via current render soft tags. |
| tests/snapshots/entry-templates.test.ts.snap | Updates generated entry templates to call setCurrentFetchSoftTags() in render and ISR regen scopes. |
| packages/vinext/src/shims/unified-request-context.ts | Adds currentFetchSoftTags to the unified per-request context defaults. |
| packages/vinext/src/shims/fetch-cache.ts | Introduces currentFetchSoftTags state, setCurrentFetchSoftTags(), and passes softTags into cache handler GETs for fetch reads. |
| packages/vinext/src/shims/cache.ts | Updates MemoryCacheHandler.get() to treat revalidated soft tags as read-time misses. |
| packages/vinext/src/entries/app-rsc-entry.ts | Threads route-derived implicit tags into fetch soft tags within the App Router request handler and ISR regeneration scopes. |
| packages/vinext/src/cloudflare/kv-cache-handler.ts | Refactors tag invalidation logic and applies soft-tag invalidation as a read-time miss (no delete) for KV cache reads. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } | ||
| } | ||
| if (await this._hasRevalidatedTag(readStringArrayField(_ctx, "softTags"), entry.lastModified)) { |
There was a problem hiding this comment.
KVCacheHandler.get() treats ctx.softTags the same as stored entry tags, but the soft tags are not validated/sanitized before being used to build KV keys in _hasRevalidatedTag(). Since revalidateTag() filters invalid tags via validateTag(), an invalid soft tag can never be revalidated and will still trigger KV reads (and potentially problematic keys). Consider validating/filtering softTags with validateTag() (or a lightweight sanitizer) before passing them to _hasRevalidatedTag, and ideally dedupe to avoid redundant KV lookups.
| if (await this._hasRevalidatedTag(readStringArrayField(_ctx, "softTags"), entry.lastModified)) { | |
| const softTags = Array.from( | |
| new Set(readStringArrayField(_ctx, "softTags").filter((tag) => validateTag(tag))), | |
| ); | |
| if (await this._hasRevalidatedTag(softTags, entry.lastModified)) { |
There was a problem hiding this comment.
Addressed in f7aaec5: KV softTags now go through the same validateTag() + dedupe helper as stored entry tags before marker lookups. I also added coverage for duplicate and invalid softTags so we only read the valid tag marker.
Fetch cache entries without explicit next.tags could survive revalidatePath(). The page or route cache entry was invalidated, but the regenerated render could still read the old fetch response and reproduce stale data. The missing boundary was Next.js soft tag semantics: path-derived implicit tags are read-time context for fetch cache lookups, not durable tags stored on the fetch entry. Thread route-derived soft tags through App Router renders, pass them to fetch cache reads, and teach the memory and KV handlers to treat revalidated soft tags as cache misses without deleting shared fetch entries. Regression coverage verifies revalidatePath invalidates untagged fetch cache reads and that KV soft-tag misses do not delete the shared entry.
09f1ce0 to
f7aaec5
Compare
|
Clarifying overlap with #709: this PR is adjacent to #709, but I do not think it is a duplicate. #709 implements the Next.js 16 This PR covers a different parity gap: So if #709 lands first, this PR may need a rebase because both touch cache handlers, but #709 does not cover the |
|
/bigbonk review |
There was a problem hiding this comment.
Nice, focused PR. The change correctly mirrors Next.js's softTags semantics: implicit path tags act as read-time invalidation only, so a revalidatePath() triggers a fetch miss in the affected render without permanently coupling the shared fetch entry to that path. Wiring is in the right places (after final route match using post-rewrite cleanPathname, and inside both regen scopes), and the KV refactor preserves the existing tag-cache + dedup behavior while adding a no-delete soft-tag check. A few small notes below — none blocking.
Verified
- Tag derivation:
__pageCacheTags(cleanPathname, [])is used at all three call sites, and matches whatrevalidatePath(path, type?)writes for the no-type,"page", and"layout"cases (including/→_N_T_/, no double-slash for root layout). - Dev/prod parity: The Pages Router does not produce App-Router-style implicit path tags, so
runWithFetchCacheresettingcurrentFetchSoftTags = []for that path is consistent. - Scope safety:
setCurrentFetchSoftTagsmutates the current unified ctx (or fallback) by replacing the array reference, not by in-place mutation, so it does not leak acrossrunWithUnifiedStateMutationparents. The two regen call sites use a fresh__revalUCtx, so they are isolated from the parent request. - Other
handler.get()callers: only the patched fetch passessoftTags. ISR page/route reads pass no ctx, so the new soft-tag branch is a no-op there. - KV refactor equivalence:
_hasRevalidatedTagpreserves the original semantics —timestamp === 0means "no marker",NaNis treated as invalidated, both passes populate the local cache before checking. Behavior for stored-tag deletion is unchanged.
Minor observations
- Asymmetric validation between Memory and KV.
MemoryCacheHandler.get()iterates rawsoftTagsfrom ctx withoutvalidateTag/dedup (KV does). For Memory this is benign since it's aMap.get(tag)lookup with no key construction, but it does mean an oversized or control-char tag would trigger unnecessary map probes. Probably not worth a separate helper; just calling out the asymmetry. _N_T_/index↔_N_T_/aliasing. Next.js'sgetImplicitTagsadds both when one is present (see implicit-tags.ts:90-99). vinext's__pageCacheTagsdoesn't, sorevalidatePath("/")won't invalidate fetches done while rendering a route that was registered as/index(and vice versa). Already called out as out of scope in the PR description.pagevs pathname for derived tags. Next.js derives layout tags from the route (/posts/[slug]/page) and adds the pathname as a separate implicit tag. vinext derives both from the resolved pathname. This is a pre-existing parity gap in__pageCacheTagsand not introduced here, but worth noting since the soft-tag implementation now depends on it for correctness —revalidatePath("/posts/[slug]")style invalidation won't match."use cache"reads don't carry softTags.cache-runtime.ts:384callshandler.get(cacheKey, { kind: "FETCH" })with nosoftTags. Out of scope here, but worth tracking — a follow-up could thread_getState().currentFetchSoftTagsthere too sorevalidatePath()invalidates"use cache"entries the same way it does fetch entries.
Test coverage looks good
tests/fetch-cache.test.tscovers the happy path throughMemoryCacheHandler.tests/kv-cache-handler.test.tscovers (a) FETCH-with-softTags-misses-without-deleting and (b) soft-tag validation/dedup. Both are the right invariants.- The 8-fixture snapshot diff (24 = 8 × 3 sites) confirms wiring is consistent across all entry templates.
|
Review posted: #917 (review) Summary of review: Verdict: solid, focused PR — no blocking issues. The change correctly mirrors Next.js's Verified correctness:
Minor non-blocking observations:
Test coverage is appropriate (Memory happy path + KV no-delete + KV validate/dedup, plus 8-fixture snapshot wiring). |
@james-elicx I wonder if we could set up bonk to auto open issues when it identified something as worth following up. |
What this changes
Route-scoped implicit tags are now threaded into App Router fetch cache reads as soft tags.
revalidatePath()can now force cachedfetch()results to miss while rendering the affected path, even when the fetch only usesnext.revalidateand no explicitnext.tags.Why
Before this change, vinext invalidated the page or route cache entry for a path, but the regenerated render could still reuse an older fetch cache entry because fetch entries only carried explicit
next.tags. Next.js treats page-derived implicit tags as read-time soft tags for fetch cache lookup:softTags: implicitTags?.tagstags + softTagsrevalidatePath()normalizes paths into_N_T_tagsApproach
Add request-scoped fetch soft tags and set them from the generated App Router entry after final route matching and inside ISR regeneration scopes. Pass those soft tags to fetch cache reads.
Memory and KV cache handlers now treat revalidated soft tags as read-time misses, but only stored entry tags trigger deletion. This keeps shared fetch entries from being permanently coupled to one route path while still matching the Next.js invalidation decision.
Validation
vp run vinext#buildvp test run tests/fetch-cache.test.ts tests/kv-cache-handler.test.ts tests/isr-cache.test.ts tests/entry-templates.test.tsvp checkRisks / follow-ups
This PR follows the existing vinext path-tag derivation helper. It does not broaden the scope to unrelated
revalidatePath()parity gaps, such as root/index alias handling.