-
Notifications
You must be signed in to change notification settings - Fork 324
fix(cache): scope use cache keys by deployment id #1092
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
20cb09d
60c8ebf
518b83e
c3cf95c
1c7d459
6c3bd8c
192ada2
6b80257
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 |
|---|---|---|
|
|
@@ -902,6 +902,11 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { | |
| // Also used to namespace ISR cache keys so old cached entries from a | ||
| // previous deploy are never served by the new one. | ||
| defines["process.env.__VINEXT_BUILD_ID"] = JSON.stringify(nextConfig.buildId); | ||
| // Deployment ID — mirrors Next.js' NEXT_DEPLOYMENT_ID seed for shared | ||
| // "use cache" entries, falling back to build ID when absent. | ||
| defines["process.env.__VINEXT_DEPLOYMENT_ID"] = JSON.stringify( | ||
| nextConfig.deploymentId ?? "", | ||
|
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. When |
||
| ); | ||
|
|
||
| // Build the shim alias map. Exact `.js` variants are included for the | ||
| // public Next entrypoints that are file-backed in `next/package.json`. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,61 +35,70 @@ export default { | |
| env?: WorkerAssetEnv, | ||
| ctx?: ExecutionContextLike, | ||
| ): Promise<Response> { | ||
| const url = new URL(request.url); | ||
| return handleRequest(request, env, ctx); | ||
| }, | ||
| }; | ||
|
|
||
| // Block protocol-relative URL open redirects (//evil.com/, /\evil.com/, | ||
| // /%5Cevil.com/, /%2F/evil.com/). Check BEFORE decode so both literal and | ||
| // percent-encoded variants are caught — encoded forms survive segment-wise | ||
| // decoding and would otherwise reach trailing-slash redirect emitters. | ||
| if (isOpenRedirectShaped(url.pathname)) { | ||
| return notFoundResponse(); | ||
| } | ||
| async function handleRequest( | ||
| request: Request, | ||
| env: WorkerAssetEnv | undefined, | ||
| ctx: ExecutionContextLike | undefined, | ||
| ): Promise<Response> { | ||
| const url = new URL(request.url); | ||
|
|
||
| // Validate that percent-encoding is well-formed. The RSC handler performs | ||
| // the actual decode + normalize; we only check here to return a clean 400 | ||
| // instead of letting a malformed sequence crash downstream. | ||
| try { | ||
| decodeURIComponent(url.pathname); | ||
| } catch { | ||
| // Malformed percent-encoding (e.g. /%E0%A4%A) — return 400 instead of throwing. | ||
| return badRequestResponse(); | ||
| } | ||
| // Block protocol-relative URL open redirects (//evil.com/, /\evil.com/, | ||
| // /%5Cevil.com/, /%2F/evil.com/). Check BEFORE decode so both literal and | ||
| // percent-encoded variants are caught — encoded forms survive segment-wise | ||
| // decoding and would otherwise reach trailing-slash redirect emitters. | ||
| if (isOpenRedirectShaped(url.pathname)) { | ||
| return notFoundResponse(); | ||
| } | ||
|
|
||
| // Strip internal headers from inbound requests before any handler or | ||
| // middleware sees them. Must happen before the RSC handler runs. | ||
| // Builds a new Headers — Request.headers is immutable in Workers. | ||
| { | ||
| const filteredHeaders = filterInternalHeaders(request.headers); | ||
| request = cloneRequestWithHeaders(request, filteredHeaders); | ||
| } | ||
| // Validate that percent-encoding is well-formed. The RSC handler performs | ||
| // the actual decode + normalize; we only check here to return a clean 400 | ||
| // instead of letting a malformed sequence crash downstream. | ||
| try { | ||
| decodeURIComponent(url.pathname); | ||
| } catch { | ||
| // Malformed percent-encoding (e.g. /%E0%A4%A) — return 400 instead of throwing. | ||
| return badRequestResponse(); | ||
| } | ||
|
|
||
| // Do NOT decode/normalize the pathname here. The RSC handler | ||
| // (virtual:vinext-rsc-entry) is the single point of decoding — it calls | ||
| // decodeURIComponent + normalizePath on the incoming URL. Decoding here | ||
| // AND in the handler would double-decode, causing inconsistent path | ||
| // matching between middleware and routing. | ||
| // Strip internal headers from inbound requests before any handler or | ||
| // middleware sees them. Must happen before the RSC handler runs. | ||
| // Builds a new Headers — Request.headers is immutable in Workers. | ||
| { | ||
| const filteredHeaders = filterInternalHeaders(request.headers); | ||
| request = cloneRequestWithHeaders(request, filteredHeaders); | ||
| } | ||
|
|
||
| // Delegate to RSC handler (which decodes + normalizes the pathname itself), | ||
| // wrapping in the ExecutionContext ALS scope so downstream code can reach | ||
| // ctx.waitUntil() without having ctx threaded through every call site. | ||
| const handleFn = () => rscHandler(request, ctx); | ||
| const result = await (ctx ? runWithExecutionContext(ctx, handleFn) : handleFn()); | ||
| // Do NOT decode/normalize the pathname here. The RSC handler | ||
| // (virtual:vinext-rsc-entry) is the single point of decoding — it calls | ||
| // decodeURIComponent + normalizePath on the incoming URL. Decoding here | ||
| // AND in the handler would double-decode, causing inconsistent path | ||
| // matching between middleware and routing. | ||
|
|
||
| if (result instanceof Response) { | ||
| if (env?.ASSETS) { | ||
| const assetResponse = await resolveStaticAssetSignal(result, { | ||
| fetchAsset: (path) => | ||
| Promise.resolve(env.ASSETS!.fetch(new Request(new URL(path, request.url)))), | ||
| }); | ||
| if (assetResponse) return assetResponse; | ||
| } | ||
| return result; | ||
| } | ||
| // Delegate to RSC handler (which decodes + normalizes the pathname itself), | ||
| // wrapping in the ExecutionContext ALS scope so downstream code can reach | ||
| // ctx.waitUntil() without having ctx threaded through every call site. | ||
| const handleFn = () => rscHandler(request, ctx); | ||
| const result = await (ctx ? runWithExecutionContext(ctx, handleFn) : handleFn()); | ||
|
|
||
| if (result === null || result === undefined) { | ||
| return notFoundResponse(); | ||
| if (result instanceof Response) { | ||
| if (env?.ASSETS) { | ||
| const assetFetcher = env.ASSETS; | ||
|
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. Nice cleanup: extracting |
||
| const assetResponse = await resolveStaticAssetSignal(result, { | ||
| fetchAsset: (path) => | ||
| Promise.resolve(assetFetcher.fetch(new Request(new URL(path, request.url)))), | ||
| }); | ||
| if (assetResponse) return assetResponse; | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| return new Response(String(result), { status: 200 }); | ||
| }, | ||
| }; | ||
| if (result === null || result === undefined) { | ||
| return notFoundResponse(); | ||
| } | ||
|
|
||
| return new Response(String(result), { status: 200 }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,7 +6,7 @@ | |||||
| * Vite plugin to wrap them with `registerCachedFunction()`. | ||||||
| * | ||||||
| * The runtime: | ||||||
| * 1. Generates a cache key from build ID + function identity + serialized arguments | ||||||
| * 1. Generates a cache key from deployment/build ID + function identity + serialized arguments | ||||||
| * 2. Checks the CacheHandler for a cached value | ||||||
| * 3. On HIT: returns the cached value (deserialized via RSC stream) | ||||||
| * 4. On MISS: creates an AsyncLocalStorage context for cacheLife/cacheTag, | ||||||
|
|
@@ -90,20 +90,34 @@ type RscModule = { | |||||
| decodeReply: (body: string | FormData, options?: unknown) => Promise<unknown[]>; | ||||||
| }; | ||||||
|
|
||||||
| function getUseCacheBuildId(): string | undefined { | ||||||
| function getUseCacheDeploymentIdDefine(): string | undefined { | ||||||
| try { | ||||||
| // Keep this direct reference so Vite's define transform can inline it for | ||||||
| // Worker bundles where the process global might not exist at runtime. A | ||||||
| // typeof process guard would return before the inlined build ID is reached. | ||||||
| // Worker bundles where the process global might not exist at runtime. | ||||||
| return process.env.__VINEXT_DEPLOYMENT_ID || process.env.NEXT_DEPLOYMENT_ID; | ||||||
| } catch (error) { | ||||||
| if (error instanceof ReferenceError) return undefined; | ||||||
| throw error; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| function getUseCacheBuildIdDefine(): string | undefined { | ||||||
| try { | ||||||
| // Keep this direct reference so Vite's define transform can inline it for | ||||||
| // Worker bundles where the process global might not exist at runtime. | ||||||
| return process.env.__VINEXT_BUILD_ID; | ||||||
| } catch (error) { | ||||||
| if (error instanceof ReferenceError) return undefined; | ||||||
| throw error; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| function buildUseCacheKey(id: string, buildId: string | undefined, argsKey?: string): string { | ||||||
| const scopedId = buildId ? `build:${encodeURIComponent(buildId)}:${id}` : id; | ||||||
| function getUseCacheKeySeed(): string | undefined { | ||||||
| return getUseCacheDeploymentIdDefine() || getUseCacheBuildIdDefine(); | ||||||
| } | ||||||
|
|
||||||
| function buildUseCacheKey(id: string, keySeed: string | undefined, argsKey?: string): string { | ||||||
| const scopedId = keySeed ? `build:${encodeURIComponent(keySeed)}:${id}` : id; | ||||||
|
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. Nit: The key prefix is still
Suggested change
|
||||||
| return argsKey === undefined ? `use-cache:${scopedId}` : `use-cache:${scopedId}:${argsKey}`; | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -332,11 +346,11 @@ export function registerCachedFunction<T extends (...args: any[]) => Promise<any | |||||
| // Per-request ("use cache: private") caching still works in dev since | ||||||
| // it's scoped to a single request and doesn't persist across HMR. | ||||||
| const isDev = typeof process !== "undefined" && process.env.NODE_ENV === "development"; | ||||||
| const buildId = getUseCacheBuildId(); | ||||||
|
|
||||||
| // oxlint-disable-next-line @typescript-eslint/no-explicit-any | ||||||
| const cachedFn = async (...args: any[]): Promise<any> => { | ||||||
| const rsc = await getRscModule(); | ||||||
| const keySeed = getUseCacheKeySeed(); | ||||||
|
|
||||||
| // Build the cache key. Use encodeReply (RSC protocol) when available — | ||||||
| // it correctly handles React elements as temporary references (excluded | ||||||
|
|
@@ -359,10 +373,10 @@ export function registerCachedFunction<T extends (...args: any[]) => Promise<any | |||||
| const encoded = await rsc.encodeReply(processedArgs, { | ||||||
| temporaryReferences: tempRefs, | ||||||
| }); | ||||||
| cacheKey = buildUseCacheKey(id, buildId, await replyToCacheKey(encoded)); | ||||||
| cacheKey = buildUseCacheKey(id, keySeed, await replyToCacheKey(encoded)); | ||||||
| } else { | ||||||
| const argsKey = args.length > 0 ? stableStringify(args) : undefined; | ||||||
| cacheKey = buildUseCacheKey(id, buildId, argsKey); | ||||||
| cacheKey = buildUseCacheKey(id, keySeed, argsKey); | ||||||
| } | ||||||
| } catch { | ||||||
| // Non-serializable arguments — run without caching | ||||||
|
|
||||||
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.
Calling out: when a user sets
deploymentId: ""in next.config.js, this silently swallowsNEXT_DEPLOYMENT_IDfrom the environment. The test covers this behavior ("treats an empty next.config.js deploymentId as unset"), so it's intentional — but it functions as a way to opt out of the env var, not just "leave unset." Worth a brief inline comment for the next person who reads this, e.g.: