-
Notifications
You must be signed in to change notification settings - Fork 297
feat: lazy per-route cache seeding for Workers #653
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
97c7a82
8011d47
8f46d7f
ee4f85b
b995b0d
7ec76af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -453,6 +453,7 @@ import { setCacheHandler } from "vinext/shims/cache"; | |||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| import { handleImageOptimization, DEFAULT_DEVICE_SIZES, DEFAULT_IMAGE_SIZES } from "vinext/server/image-optimization"; | ||||||||||||||||||||||||
| import type { ImageConfig } from "vinext/server/image-optimization"; | ||||||||||||||||||||||||
| import { seedRouteFromAssets } from "vinext/server/seed-cache-workers"; | ||||||||||||||||||||||||
| import handler from "vinext/server/app-router-entry"; | ||||||||||||||||||||||||
| ${isrImports} | ||||||||||||||||||||||||
| interface Env { | ||||||||||||||||||||||||
|
|
@@ -495,6 +496,11 @@ ${isrSetup} const url = new URL(request.url); | |||||||||||||||||||||||
| }, allowedWidths); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Seed the memory cache from pre-rendered assets (lazy, per-route). | ||||||||||||||||||||||||
| // Blocking (not waitUntil) so the RSC handler sees the seeded cache. | ||||||||||||||||||||||||
| // seedRouteFromAssets handles errors internally — never throws. | ||||||||||||||||||||||||
| await seedRouteFromAssets(url.pathname, (p) => env.ASSETS.fetch(new Request(new URL(p, request.url)))); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
Comment on lines
498
to
+503
|
||||||||||||||||||||||||
| // Seed the memory cache from pre-rendered assets (lazy, per-route). | |
| // Blocking (not waitUntil) so the RSC handler sees the seeded cache. | |
| // seedRouteFromAssets handles errors internally — never throws. | |
| await seedRouteFromAssets(url.pathname, (p) => env.ASSETS.fetch(new Request(new URL(p, request.url)))); | |
| ${hasISR ? "" : ` | |
| // Seed the memory cache from pre-rendered assets (lazy, per-route). | |
| // Blocking (not waitUntil) so the RSC handler sees the seeded cache. | |
| // seedRouteFromAssets handles errors internally — never throws. | |
| await seedRouteFromAssets(url.pathname, (p) => env.ASSETS.fetch(new Request(new URL(p, request.url)))); | |
| `} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: fs.cpSync(sourceDir, targetDir, { recursive: true }) copies the contents of sourceDir into targetDir where targetDir already exists. This means vinext-prerender.json (copied on line 1219) and route files (e.g., about.html) all live at the same level under __prerender/. This works, but means a pre-rendered route literally named /vinext-prerender (producing vinext-prerender.html) would coexist fine — just calling it out.
A more robust layout would be __prerender/manifest.json + __prerender/routes/..., but that's a future cleanup, not a blocker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Observation: fs.cpSync(sourceDir, targetDir) merges the contents of prerendered-routes/ into __prerender/, which already contains vinext-prerender.json. Since route files are .html/.rsc and the manifest is .json, there's no collision risk. But if a future route produces a .json output file, it could theoretically overwrite the manifest. Very unlikely, but a __prerender/manifest/ vs __prerender/routes/ split would make this impossible. Low priority.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| /** | ||
| * Shared types and helpers for seed-cache modules (Node.js and Workers). | ||
| * | ||
| * Both `seed-cache.ts` (eager, fs-based) and `seed-cache-workers.ts` | ||
| * (lazy, fetch-based) read the same vinext-prerender.json manifest and | ||
| * produce identical cache entries. This module holds the shared contract. | ||
| */ | ||
|
|
||
| import type { CachedAppPageValue } from "../shims/cache.js"; | ||
|
|
||
| // ─── Manifest types ────────────────────────────────────────────────────────── | ||
|
|
||
| export type PrerenderManifest = { | ||
| buildId: string; | ||
| basePath?: string; | ||
| trailingSlash?: boolean; | ||
| routes: PrerenderManifestRoute[]; | ||
| }; | ||
|
|
||
| export type PrerenderManifestRoute = { | ||
| route: string; | ||
| status: string; | ||
| revalidate?: number | false; | ||
| path?: string; | ||
| router?: "app" | "pages"; | ||
| }; | ||
|
|
||
| // ─── Cache value construction ──────────────────────────────────────────────── | ||
|
|
||
| /** | ||
| * Build the CacheHandler context object from a revalidate value. | ||
| * `revalidate: undefined` (static routes) → empty context → no expiry. | ||
| */ | ||
| export function revalidateCtx(seconds: number | undefined): Record<string, unknown> { | ||
| return seconds !== undefined ? { revalidate: seconds } : {}; | ||
| } | ||
|
|
||
| /** Build an APP_PAGE cache value for an HTML entry. */ | ||
| export function makeHtmlCacheValue(html: string): CachedAppPageValue { | ||
| return { | ||
| kind: "APP_PAGE", | ||
| html, | ||
| rscData: undefined, | ||
| headers: undefined, | ||
| postponed: undefined, | ||
| status: undefined, | ||
| }; | ||
| } | ||
|
|
||
| /** Build an APP_PAGE cache value for an RSC entry. */ | ||
| export function makeRscCacheValue(rscData: ArrayBuffer): CachedAppPageValue { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth noting for future readers: |
||
| return { | ||
| kind: "APP_PAGE", | ||
| html: "", | ||
| rscData, | ||
| headers: undefined, | ||
| postponed: undefined, | ||
| status: undefined, | ||
| }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This
awaitis on the critical path of every request — it blocks the response until seeding completes for pre-rendered routes. For the first cold request this adds the latency of twoenv.ASSETS.fetch()calls (HTML + RSC) plus twohandler.set()calls.This is probably acceptable since
env.ASSETS.fetch()is local to the colo and fast, but worth a comment noting that this is intentionally blocking (to guarantee the cache is populated before the RSC handler runs). An alternative would be to let the RSC handler serve the request normally while seeding in the background viactx.waitUntil(), but that would mean the first request always re-renders — so blocking is the right call here.Also: should this be wrapped in try/catch as defense-in-depth? If
seedRouteFromAssetsthrows (see other comment), the request dies.