feat(app-router): skip core cache writes without a negative searchParams proof - #3456
james-elicx wants to merge 3 commits into
Conversation
|
/bigbonk review for issues Please review exact head |
@vinext/cloudflare
create-vinext-app
@vinext/types
vinext
@cloudflare/workers-response-store
commit: |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
| await expect(response.text()).resolves.toBe("<h1>fresh</h1>"); | ||
| await proven.text(); | ||
| await Promise.all(pendingCacheWrites); | ||
| expect(isrSet.mock.calls.map(([key]) => key)).toEqual(["html:/fresh", "rsc:/fresh"]); |
There was a problem hiding this comment.
vp check fails here with TS2493 because vi.fn(async () => {}) infers a zero-argument mock, so each recorded call has tuple type [] and cannot be destructured as [key]. Please type the mock with AppPageCacheSetter (or give the callback the setter parameters) so the required check passes.
There was a problem hiding this comment.
Fixed in cfffc4f. The mock is now typed as vi.fn<AppPageCacheSetter>(async () => {}), so its recorded calls destructure as [key].
|
Submitted 1 inline finding. Focused tests pass; |
Performance benchmarksCompared 0 improved · 0 regressed · 6 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head |
e1877bf to
cfffc4f
Compare
|
/bigbonk review for issues Please review exact head |
|
|
LGTM! |
cfffc4f to
c864935
Compare
c864935 to
7e47900
Compare
|
/bigbonk review for issues Please review exact head |
|
LGTM! |
7e47900 to
d6e9d38
Compare
|
/bigbonk review for issues Please review exact head |
|
LGTM! |
d6e9d38 to
037b17b
Compare
|
/bigbonk review for issues Please review exact head |
|
LGTM! |
037b17b to
f69afb4
Compare
|
LGTM! |
99264c9 to
e21ad6d
Compare
|
/bigbonk review for issues Please review exact head |
|
LGTM! |
e21ad6d to
bcc0485
Compare
|
/bigbonk review for issues Please review exact head |
|
LGTM! |
bcc0485 to
91fdf78
Compare
|
/bigbonk review for issues Please review exact head |
|
LGTM! |
91fdf78 to
71d5de5
Compare
|
/bigbonk review for issues Please review exact head |
|
LGTM! |
71d5de5 to
161dc34
Compare
|
/bigbonk review for issues Please review exact head |
|
LGTM! |
161dc34 to
49ceec8
Compare
|
/bigbonk review for issues Please review exact head |
|
LGTM! |
…ams proof Every query shares an App page's HTML and RSC entries, so core's five request-time and regeneration writes now store a render only when its observation proves searchParams went unread. The observation builders are required, since a write path without one is a bug.
…ery-invariant proof
49ceec8 to
41a9c1a
Compare
1 participant
Stacked on #3457. Third part of plan PR 1 (cache-candidate SSR mode): the safety net in core's writes. It sits on the client-page PR (#3457), so a static client page that never reads
searchParamsstill has a proof and is still stored.Problem
Core stores one HTML entry and one RSC entry per App page, and every query shares them. The read side already refuses to serve a stored entry to a query-bearing request unless its render observation proves
searchParamswent unread. The write side had no matching check, so a render that read the query could still be stored under the query-free key.Change
Core's five request-time and regeneration writes now store a render only when its observation has a complete negative
searchParamsproof (hasQueryInvariantRenderProof, built onhasCompleteNegativeRequestApiProof):Response headers don't change. The read side reuses the same helper.
The observation builders (
createHtmlRenderObservation,createRscRenderObservation) and the regeneration result's observations are now required, because a write path that supplies no observation is a bug.A regeneration that reads
searchParamsis no longer stored. It renders with an empty query, and Next.js treats asearchParamsread as dynamic, so it never stores that render either.Tests
tests/app-page-cache.test.ts: a render that readsearchParamsskips each write (HTML finalizer for either observation, RSC-only, regeneration for HTML, RSC, and an RSC-triggered regen), and the HTML finalizer's response headers match the stored case. Mutating the helper to always pass fails all of them.tests/app-page-cache-render.test.ts: a static regeneration's real HTML and RSC observations carry the proof, so it's still stored.tests/app-page-dispatch.test.ts: a stale regeneration that readssearchParamsin the page or in metadata writes nothing.app-page-renderandapp-page-dispatchstill write with real observations.examples/response-store-demo) passes: 26 tests.