Skip to content

Commit e19bb47

Browse files
authored
fix(ppr): collect shell tags at the putShell barrier so async-recorded tags evict the shell (#676) (#680)
1 parent 7dd3ba5 commit e19bb47

5 files changed

Lines changed: 200 additions & 29 deletions

File tree

‎packages/rangojs-router/docs/design/ppr-shell-resume.md‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -363,17 +363,21 @@ The expiry invariant holds BY CONSTRUCTION, no filtering logic:
363363
capture (its loaders never run), so nothing under a hole can tag the shell; it
364364
stays live and re-renders per request regardless of tag invalidation.
365365

366-
Timing: the capture snapshots the tag union (`attemptCapture`) right after it
367-
kicks off the shell render. React renders a SYNCHRONOUS server component before
368-
`renderToReadableStream` returns, so a `cacheTag()` in a synchronous component —
369-
the #648 case, verified by the cloudflare-basic eviction e2e — is on
370-
`_requestTags` in time. A tag recorded only AFTER an `await` inside an async
371-
component lands after that snapshot and is not collected into the shell (a known
372-
edge, shared with async `cache()`/`"use cache"` reads at capture; the document
373-
cache does not share it — it buffers the full response body before
374-
`collectRequestTags`). Deferring the shell's tag snapshot to a render-complete
375-
barrier is a viable follow-up if async render-recorded shell tags become load-
376-
bearing.
366+
Timing: the tag snapshot sits at the putShell WRITE BARRIER (`captureAndStoreShell`,
367+
right before it builds the `ShellCacheEntry`), not at stream construction. By the
368+
barrier the capture has already quiesced — handles settled, Flight task-quiet — and
369+
the deferred cache writes were awaited, so any tag the render recorded onto
370+
`_requestTags` is on the set: a `cacheTag()` in a synchronous server component (the
371+
#648 case) AND a tag recorded only AFTER an `await` inside an async server component
372+
(#676), plus tags propagated by async `cache()`/`"use cache"` reads at capture
373+
(their `recordRequestTags` can post-date the render). This mirrors the document
374+
cache, which buffers the full response body before `collectRequestTags`. The earlier
375+
`attemptCapture` snapshot — taken right after `renderToReadableStream` returned,
376+
before React had rendered anything past the first await — dropped those late tags
377+
silently; moving it behind the quiesce gate closed that window (issue #676). Holes
378+
are unaffected by the move: a masked loader never executes during capture, so
379+
nothing under a hole records a tag regardless of when the snapshot runs — the
380+
baked ⇒ evicts / hole ⇒ fresh invariant above still holds by construction.
377381

378382
### The handles contract: "nesting = liveness"
379383

‎packages/rangojs-router/src/rsc/__tests__/shell-capture.test.ts‎

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717
type RequestContext,
1818
} from "../../server/request-context.js";
1919
import { MemorySegmentCacheStore } from "../../cache/memory-segment-store.js";
20-
import { cacheTag } from "../../cache/cache-tag.js";
20+
import { cacheTag, recordRequestTags } from "../../cache/cache-tag.js";
2121
import type { HandlerContext } from "../handler-context.js";
2222
import type { SSRModule } from "../types.js";
2323

@@ -241,6 +241,10 @@ describe("captureAndStoreShell", () => {
241241
return {
242242
_cacheStore: putShell ? { putShell } : undefined,
243243
_reportBackgroundError: vi.fn(),
244+
// The real derived context always seeds a fresh _requestTags (shell-capture
245+
// attemptCapture); the putShell write barrier snapshots it (issue #676), so
246+
// the direct-call stub must model the same required RequestContext field.
247+
_requestTags: new Set<string>(),
244248
};
245249
}
246250

@@ -1258,6 +1262,93 @@ describe("runShellCapture", () => {
12581262
expect(await store.getShell("/p:shell")).toBeNull();
12591263
});
12601264

1265+
// #676: a cacheTag() recorded AFTER an await inside an async shell server
1266+
// component. React renders the synchronous tree during RSC-stream construction,
1267+
// so a sync cacheTag lands; a tag recorded on a later microtask/macrotask (past
1268+
// the await) lands after construction. The shell tag snapshot now sits at the
1269+
// putShell WRITE BARRIER — after the capture quiesces — so the late tag is
1270+
// collected, unioned with the static ppr.tags, and revalidateTag() evicts the
1271+
// shell. captureShellHTML gates on the same promise the component awaits, so the
1272+
// tag records within the capture's quiesce window (the real async component the
1273+
// shell waits for).
1274+
it("collects a cacheTag() recorded after an await in async shell content (#676)", async () => {
1275+
const putShell = makePutShell();
1276+
let recorded: Promise<void> = Promise.resolve();
1277+
const { ctx, ssrModule } = makeCtx(
1278+
okMatch,
1279+
vi.fn(async () => {
1280+
await recorded;
1281+
return { prelude: enc("<body>x</body>"), postponed: null };
1282+
}),
1283+
);
1284+
(ctx as any).renderToReadableStream = vi.fn(() => {
1285+
recorded = (async () => {
1286+
await new Promise((r) => setTimeout(r, 0));
1287+
cacheTag("async-shell-tag");
1288+
})();
1289+
return emptyStream();
1290+
});
1291+
1292+
await runShellCapture(
1293+
ctx,
1294+
new Request("http://localhost/p"),
1295+
{},
1296+
new URL("http://localhost/p"),
1297+
makeReqCtx(),
1298+
ssrModule,
1299+
{ key: "/p:shell", store: { putShell } as any, tags: ["ppr:static"] },
1300+
0,
1301+
);
1302+
1303+
expect(putShell).toHaveBeenCalledTimes(1);
1304+
const tags = new Set(putShell.mock.calls[0]![4]);
1305+
expect(tags).toEqual(new Set(["ppr:static", "async-shell-tag"]));
1306+
});
1307+
1308+
// #676 (async "use cache" propagation): a "use cache" read that HITs during the
1309+
// capture render propagates its entry tags to the document artifact via the
1310+
// cache runtime's recordRequestTags(entry.tags) — and an async cached function
1311+
// records them AFTER its await resolves. Simulate that timing: the render kicks
1312+
// off a delayed continuation that recordRequestTags() after an await, gated the
1313+
// same way the cache read-HIT test gates on the read. The late tag must reach
1314+
// the stored entry, same as the render-callable cacheTag form above.
1315+
it("collects an async use-cache read's tags recorded after an await (#676)", async () => {
1316+
const putShell = makePutShell();
1317+
let recorded: Promise<void> = Promise.resolve();
1318+
const { ctx, ssrModule } = makeCtx(
1319+
okMatch,
1320+
vi.fn(async () => {
1321+
await recorded;
1322+
return { prelude: enc("<body>x</body>"), postponed: null };
1323+
}),
1324+
);
1325+
(ctx as any).renderToReadableStream = vi.fn(() => {
1326+
const reqCtx = getRequestContext();
1327+
recorded = (async () => {
1328+
await new Promise((r) => setTimeout(r, 0));
1329+
// What cache-runtime.ts does on a "use cache" read-HIT: propagate the
1330+
// resolved entry's tags to the request's document artifact.
1331+
recordRequestTags(["async-use-cache-tag"], reqCtx);
1332+
})();
1333+
return emptyStream();
1334+
});
1335+
1336+
await runShellCapture(
1337+
ctx,
1338+
new Request("http://localhost/p"),
1339+
{},
1340+
new URL("http://localhost/p"),
1341+
makeReqCtx(),
1342+
ssrModule,
1343+
{ key: "/p:shell", store: { putShell } as any },
1344+
0,
1345+
);
1346+
1347+
expect(putShell).toHaveBeenCalledTimes(1);
1348+
const tags = new Set(putShell.mock.calls[0]![4]);
1349+
expect(tags.has("async-use-cache-tag")).toBe(true);
1350+
});
1351+
12611352
// Capture data snapshot: a cache read-HIT the capture render performs through
12621353
// the ambient (recording) store is recorded onto entry.snapshot, so a HIT can
12631354
// replay it and match the frozen prelude. See cache/shell-snapshot.ts.

‎packages/rangojs-router/src/rsc/shell-capture.ts‎

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -487,9 +487,12 @@ export function gateFlightForCapture(
487487
* store, and passed to scheduleShellCapture directly — it is NOT threaded through
488488
* the request context. `tags` carries the route's OPERATIONAL `ppr.tags`; the
489489
* capture UNIONS them with the shell's own auto-collected (non-loader) request
490-
* tags from its derived render (the collected set stays authoritative). `store`
491-
* is the same store the serve path resolved for its getShell read
492-
* (requestCtx._cacheStore), so the capture writes where the serve reads.
490+
* tags from its derived render (the collected set stays authoritative). That
491+
* union happens at the putShell WRITE BARRIER in captureAndStoreShell — after the
492+
* capture quiesces — not at stream construction, so a tag recorded after an await
493+
* in async shell content is still collected (issue #676). `store` is the same
494+
* store the serve path resolved for its getShell read (requestCtx._cacheStore),
495+
* so the capture writes where the serve reads.
493496
*/
494497
export interface ShellCaptureDescriptor {
495498
key: string;
@@ -776,24 +779,18 @@ async function attemptCapture(
776779
},
777780
});
778781

779-
// Shell tags = the non-loader request tags the capture render recorded on its
780-
// own fresh _requestTags (loaders are masked, so loader cache tags — which
781-
// belong to the holes, not the shell — are correctly excluded), UNIONED with
782-
// the middleware's operational `tags` option (descriptor.tags). The collected
783-
// set is authoritative; the option only adds tags the render cannot know.
784-
const collected = [...derivedCtx._requestTags];
785-
const union = new Set<string>([...(descriptor.tags ?? []), ...collected]);
786-
const tags = union.size > 0 ? [...union] : undefined;
787-
782+
// Pass the descriptor with its STATIC ppr.tags unchanged. The shell's own
783+
// render-recorded tags are snapshotted at the putShell WRITE BARRIER inside
784+
// captureAndStoreShell, not here: a tag recorded AFTER an await in async shell
785+
// content (and tags propagated by async cache()/"use cache" reads) lands after
786+
// this synchronous construction point, so snapshotting here dropped it — the
787+
// shell-tag snapshot must sit behind the quiesce gate (issue #676).
788788
return captureAndStoreShell(
789789
ssrModule,
790790
rscStream,
791791
freshHandleStore,
792792
derivedCtx,
793-
{
794-
...descriptor,
795-
tags,
796-
},
793+
descriptor,
797794
);
798795
});
799796
}
@@ -1035,6 +1032,19 @@ async function captureAndStoreShell(
10351032
}
10361033
}
10371034

1035+
// Shell tags snapshot at the WRITE BARRIER, not at stream construction: by
1036+
// here the capture has quiesced and the deferred cache writes were awaited, so
1037+
// tags recorded AFTER an await in async shell content (and by async
1038+
// cache()/"use cache" reads propagating through recordRequestTags) are
1039+
// included — issue #676. Loaders are masked, so loader cache tags — which
1040+
// belong to the holes, not the shell — never execute during capture and cannot
1041+
// contribute. Union with the route's static ppr.tags (capture.tags); the
1042+
// collected set is authoritative, the option only adds what the render cannot
1043+
// know.
1044+
const collected = [...reqCtx._requestTags];
1045+
const union = new Set<string>([...(capture.tags ?? []), ...collected]);
1046+
const shellTags = union.size > 0 ? [...union] : undefined;
1047+
10381048
const store = capture.store ?? reqCtx._cacheStore;
10391049
if (store?.putShell) {
10401050
try {
@@ -1058,7 +1068,7 @@ async function captureAndStoreShell(
10581068
entry,
10591069
capture.ttl,
10601070
capture.swr,
1061-
capture.tags,
1071+
shellTags,
10621072
);
10631073
} catch (error) {
10641074
// Best-effort: a failed put must never throw out of the background task.

‎tests/cloudflare-basic/e2e/ppr-tag-eviction.test.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,50 @@ function describeTagEviction(mode: "dev" | "build") {
8888
await warmToHit(request, url);
8989
});
9090

91+
test("a shell tagged AFTER an await in an async server component is evicted by updateTag(), then recaptures (#676)", async ({
92+
request,
93+
}) => {
94+
// The async twin of the sync case above: BlogLayout renders AsyncShellTagger,
95+
// an async bake-lane server component that records its cacheTag() only after
96+
// an await. Before the write-barrier snapshot moved behind the quiesce gate,
97+
// that post-await tag was dropped from the shell entry and this invalidation
98+
// could not evict — the exact #676 loss. Distinct probe/param => distinct
99+
// shell key and tag, so it never touches the sync case or a sibling shell.
100+
const probe = uniqueProbe("async-evict");
101+
const tag = `pprblog-async-shell-${probe}`;
102+
const url = f.url(`/ppr-blog?asyncshelltag=${probe}`);
103+
104+
// 1. Prime: virgin shell key MISSes; the background capture records the
105+
// async-recorded cacheTag(tag) into the shell entry (only reachable because
106+
// the snapshot now runs at the putShell write barrier, post-quiesce).
107+
const first = await request.get(url, { headers: HTML_HEADERS });
108+
expect(first.status()).toBe(200);
109+
expect(first.headers()["x-rango-shell"]).toBe("MISS");
110+
111+
// 2. Warm until the capture lands as a HIT.
112+
await warmToHit(request, url);
113+
114+
// 3. Invalidate the async-recorded tag.
115+
const inv = await request.get(f.url(`/test/invalidate-tag/${tag}`));
116+
expect(inv.status()).toBe(200);
117+
118+
// 4. The shell carries the async tag => it is evicted; the next document GET
119+
// MISSes (recapture). Poll to absorb KV marker propagation on the dev worker.
120+
await expect
121+
.poll(
122+
async () =>
123+
(await request.get(url, { headers: HTML_HEADERS })).headers()[
124+
"x-rango-shell"
125+
],
126+
{ timeout: 15000 },
127+
)
128+
.toBe("MISS");
129+
130+
// 5. It recaptures and HITs again (re-tagged), so the async tag keeps
131+
// tracking the shell across generations.
132+
await warmToHit(request, url);
133+
});
134+
91135
test("an untagged sibling probe shell survives a different tag's invalidation (isolation)", async ({
92136
request,
93137
}) => {

‎tests/cloudflare-basic/src/pages/blog.tsx‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,22 @@ import {
99
type BlogSidebarData,
1010
} from "../loaders/blog.js";
1111

12+
// #676 render-callable cacheTag AFTER an await: an ASYNC bake-lane server
13+
// component. Regular server components are NOT masked at capture (only loaders
14+
// are), so this runs during the shell capture; its tag records past the await —
15+
// after the synchronous RSC construction where the shell tag snapshot used to
16+
// sit. The snapshot now sits at the putShell write barrier (after the capture
17+
// quiesces), so this late tag is still collected and updateTag() drops the shell.
18+
// Gated (via BlogLayout's `asyncshelltag` probe) so only the async-eviction e2e
19+
// triggers it — never /blog, never a sibling shell.
20+
async function AsyncShellTagger({ probe }: { probe: string }) {
21+
await Promise.resolve();
22+
// Colon-free tag: it round-trips through the /test/invalidate-tag/:tag URL
23+
// param without a ":" URL-encoding into a mismatch.
24+
cacheTag(`pprblog-async-shell-${probe}`);
25+
return null;
26+
}
27+
1228
export function BlogLayout(ctx: HandlerContext) {
1329
// #648 render-callable cacheTag: this baked server component tags the PPR shell
1430
// it renders into — NO cache()/"use cache" anywhere in its tree — so
@@ -24,12 +40,18 @@ export function BlogLayout(ctx: HandlerContext) {
2440
cacheTag(`pprblog-shell-${shellProbe}`);
2541
}
2642

43+
// #676 async variant: the tag is recorded AFTER an await inside AsyncShellTagger,
44+
// not synchronously here. Gated on its own probe so its shell key and tag are
45+
// distinct from the sync `shelltag` case and any sibling shell.
46+
const asyncShellProbe = ctx.searchParams.get("asyncshelltag");
47+
2748
const breadcrumb = ctx.use(Breadcrumbs);
2849
breadcrumb({ label: "Home", href: ctx.reverse("home") });
2950
breadcrumb({ label: "Blog", href: ctx.reverse("blog") });
3051

3152
return (
3253
<div data-testid="blog-layout" style={{ display: "flex", gap: "2rem" }}>
54+
{asyncShellProbe ? <AsyncShellTagger probe={asyncShellProbe} /> : null}
3355
<main style={{ flex: 1 }}>
3456
<Outlet />
3557
</main>

0 commit comments

Comments
 (0)