refactor: share Pages ISR cache runtime#684
refactor: share Pages ISR cache runtime#684southpolesteve wants to merge 1 commit intocodex/pages-api-route-runtimefrom
Conversation
|
/bigbonk |
There was a problem hiding this comment.
Clean refactor that does exactly what the AGENTS.md guidelines prescribe: move real behavior out of the generated entry into a typed, testable module, keeping the codegen layer as thin delegation glue.
Verified:
-
getRequestExecutionContextidentity -- The generated entry imports fromshims/request-context.js(aliased as_getRequestExecutionContext), andserver/isr-cache.tsimports the same function from../shims/request-context.js. Both resolve to the same module. The shared module'sgetRequestExecutionContext()?.waitUntil(promise)is equivalent to the old inlineif (ctx && typeof ctx.waitUntil === "function") ctx.waitUntil(promise)pattern. -
tagsdefault handling -- The old inlineisrSetusedtags || []. The sharedisrSetusestags ?? []. Both produce[]forundefined/nullinputs. No behavioral regression. -
isrCacheKeybuildId injection -- The thin wrapper correctly passesbuildId || undefined, matching the old inline behavior wherebuildIdwas a top-level const that could benull. The|| undefinedcoercion ensures the shared function's optional parameter semantics work correctly. -
pendingRegenerationsscope -- The old inline code used a module-scopednew Map(). The shared module usesSymbol.for()onglobalThisfor cross-environment dedup. This is strictly better for the Pages Router case since it means the dedup map is shared if the module is loaded in multiple Vite environments. -
Tests -- Assertions correctly shifted from checking inline implementation details (regex for
renderFn()ordering,waitUntilplacement,fnv1a64) to checking delegation patterns (import presence, wrapper function shapes, absence of removed code).
One observation (not blocking): the App Router entry still inlines its own ISR helpers with a comment explaining why (RSC environment module-evaluation constraints). A follow-up to share those too would further reduce duplication, but the comment documents the constraint clearly.
|
Review posted and PR approved. Here's a summary of what I verified: PR #684 swaps the Pages Router server entry from an inline ISR implementation to delegating to the shared Key verification points:
|
Summary
server/isr-cache.tsruntime instead of embedding a second ISR implementation in the generated templateisrGet,isrSet,triggerBackgroundRegeneration, andisrCacheKeythrough typed helpersVerification
vp check packages/vinext/src/entries/pages-server-entry.ts tests/entry-templates.test.tsvp test run tests/entry-templates.test.ts tests/isr-cache.test.ts -uvp test run tests/pages-router.test.ts -t "renders pages with getStaticPaths + getStaticProps|returns 404 for paths not in getStaticPaths when fallback is false|keeps ISR cache-fill rerenders isolated from the streamed render state|wraps stale regeneration in a fresh unified request context"vp run vinext#buildWritten by Codex.