Skip to content

Include buildId in the cache breaker until we have hashed Action IDs #70542

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

Merged
merged 3 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1904,6 +1904,7 @@ export default async function build(
nextConfigOutput: config.output,
pprConfig: config.experimental.ppr,
isAppPPRFallbacksEnabled: config.experimental.pprFallbacks,
buildId,
})
)

Expand Down Expand Up @@ -2123,6 +2124,7 @@ export default async function build(
pprConfig: config.experimental.ppr,
isAppPPRFallbacksEnabled:
config.experimental.pprFallbacks,
buildId,
})
}
)
Expand Down
6 changes: 6 additions & 0 deletions packages/next/src/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1360,6 +1360,7 @@ export async function buildAppStaticPaths({
ComponentMod,
isRoutePPREnabled,
isAppPPRFallbacksEnabled,
buildId,
}: {
dir: string
page: string
Expand All @@ -1376,6 +1377,7 @@ export async function buildAppStaticPaths({
ComponentMod: AppPageModule
isRoutePPREnabled: boolean | undefined
isAppPPRFallbacksEnabled: boolean | undefined
buildId: string
}): Promise<PartialStaticPathsResult> {
ComponentMod.patchFetch()

Expand Down Expand Up @@ -1424,6 +1426,7 @@ export async function buildAppStaticPaths({
after: false,
dynamicIO: false,
},
buildId,
},
},
async (): Promise<PartialStaticPathsResult> => {
Expand Down Expand Up @@ -1592,6 +1595,7 @@ export async function isPageStatic({
cacheHandler,
pprConfig,
isAppPPRFallbacksEnabled,
buildId,
}: {
dir: string
page: string
Expand All @@ -1613,6 +1617,7 @@ export async function isPageStatic({
nextConfigOutput: 'standalone' | 'export' | undefined
pprConfig: ExperimentalPPRConfig | undefined
isAppPPRFallbacksEnabled: boolean | undefined
buildId: string
}): Promise<PageIsStaticResult> {
const isPageStaticSpan = trace('is-page-static-utils', parentId)
return isPageStaticSpan
Expand Down Expand Up @@ -1739,6 +1744,7 @@ export async function isPageStatic({
nextConfigOutput,
isRoutePPREnabled,
isAppPPRFallbacksEnabled,
buildId,
}))
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ export interface StaticGenerationStore {
isPrefetchRequest?: boolean

requestEndedState?: { ended?: boolean }

buildId: string
}

export type StaticGenerationAsyncStorage =
Expand Down
4 changes: 3 additions & 1 deletion packages/next/src/export/routes/app-route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ export async function exportAppRoute(
distDir: string,
htmlFilepath: string,
fileWriter: FileWriter,
experimental: Required<Pick<ExperimentalConfig, 'after' | 'dynamicIO'>>
experimental: Required<Pick<ExperimentalConfig, 'after' | 'dynamicIO'>>,
buildId: string
): Promise<ExportRouteResult> {
// Ensure that the URL is absolute.
req.url = `http://localhost:3000${req.url}`
Expand Down Expand Up @@ -76,6 +77,7 @@ export async function exportAppRoute(
incrementalCache,
waitUntil: undefined,
onClose: undefined,
buildId,
},
}

Expand Down
3 changes: 2 additions & 1 deletion packages/next/src/export/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,8 @@ async function exportPageImpl(
distDir,
htmlFilepath,
fileWriter,
input.renderOpts.experimental
input.renderOpts.experimental,
input.renderOpts.buildId
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export type StaticGenerationContext = {
| 'nextExport'
| 'isDraftMode'
| 'isDebugDynamicAccesses'
| 'buildId'
> &
Partial<RequestLifecycleOpts>
}
Expand Down Expand Up @@ -114,6 +115,7 @@ export const withStaticGenerationStore: WithStore<

requestEndedState,
isPrefetchRequest,
buildId: renderOpts.buildId,
}

// TODO: remove this when we resolve accessing the store outside the execution context
Expand Down
1 change: 1 addition & 0 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2506,6 +2506,7 @@ export default abstract class Server<
onClose: res.onClose.bind(res),
onInstrumentationRequestError:
this.renderOpts.onInstrumentationRequestError,
buildId: this.renderOpts.buildId,
},
}

Expand Down
1 change: 1 addition & 0 deletions packages/next/src/server/dev/next-dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,7 @@ export default class DevServer extends Server {
maxMemoryCacheSize: this.nextConfig.cacheMaxMemorySize,
nextConfigOutput: this.nextConfig.output,
isAppPPRFallbacksEnabled: this.nextConfig.experimental.pprFallbacks,
buildId: this.renderOpts.buildId,
})
return pathsResult
} finally {
Expand Down
3 changes: 3 additions & 0 deletions packages/next/src/server/dev/static-paths-worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export async function loadStaticPaths({
cacheHandler,
nextConfigOutput,
isAppPPRFallbacksEnabled,
buildId,
}: {
dir: string
distDir: string
Expand All @@ -70,6 +71,7 @@ export async function loadStaticPaths({
cacheHandler?: string
nextConfigOutput: 'standalone' | 'export' | undefined
isAppPPRFallbacksEnabled: boolean | undefined
buildId: string
}): Promise<PartialStaticPathsResult> {
// update work memory runtime-config
require('../../shared/lib/runtime-config.external').setConfig(config)
Expand Down Expand Up @@ -129,6 +131,7 @@ export async function loadStaticPaths({
nextConfigOutput,
isRoutePPREnabled,
isAppPPRFallbacksEnabled,
buildId,
})
}

Expand Down
42 changes: 30 additions & 12 deletions packages/next/src/server/use-cache/use-cache-wrapper.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { DeepReadonly } from '../../shared/lib/deep-readonly'
import { createSnapshot } from '../../client/components/async-local-storage'
/* eslint-disable import/no-extraneous-dependencies */
import {
Expand All @@ -15,6 +16,8 @@ import {
import type { StaticGenerationStore } from '../../client/components/static-generation-async-storage.external'
import { staticGenerationAsyncStorage } from '../../client/components/static-generation-async-storage.external'

import type { ClientReferenceManifest } from '../../build/webpack/plugins/flight-manifest-plugin'

import {
getClientReferenceManifestSingleton,
getServerModuleMap,
Expand Down Expand Up @@ -75,14 +78,15 @@ const runInCleanSnapshot = createSnapshot()

async function generateCacheEntry(
staticGenerationStore: StaticGenerationStore,
clientReferenceManifest: DeepReadonly<ClientReferenceManifest>,
cacheHandler: CacheHandler,
serializedCacheKey: string | ArrayBuffer,
encodedArguments: FormData | string,
fn: any
): Promise<ReadableStream> {
const temporaryReferences = createServerTemporaryReferenceSet()

const [, args] = await decodeReply<any[]>(
const [, , args] = await decodeReply<any[]>(
encodedArguments,
getServerModuleMap(),
{
Expand All @@ -96,11 +100,9 @@ async function generateCacheEntry(
let didError = false
let firstError: any = null

const clientReferenceManifestSingleton = getClientReferenceManifestSingleton()

const stream = renderToReadableStream(
result,
clientReferenceManifestSingleton.clientModules,
clientReferenceManifest.clientModules,
{
environmentName: 'Cache',
temporaryReferences,
Expand Down Expand Up @@ -172,9 +174,22 @@ export function cache(kind: string, id: string, fn: any) {
const name = fn.name
const cachedFn = {
[name]: async function (...args: any[]) {
const staticGenerationStore = staticGenerationAsyncStorage.getStore()
if (staticGenerationStore === undefined) {
throw new Error(
'"use cache" cannot be used outside of App Router. Expected a StaticGenerationStore.'
)
}

// Because the Action ID is not yet unique per implementation of that Action we can't
// safely reuse the results across builds yet. In the meantime we add the buildId to the
// arguments as a seed to ensure they're not reused. Remove this once Action IDs hash
// the implementation.
const buildId = staticGenerationStore.buildId

const temporaryReferences = createClientTemporaryReferenceSet()
const encodedArguments: FormData | string = await encodeReply(
[id, args],
[buildId, id, args],
{
temporaryReferences,
}
Expand All @@ -193,13 +208,6 @@ export function cache(kind: string, id: string, fn: any) {
let entry: undefined | CacheEntry =
await cacheHandler.get(serializedCacheKey)

const staticGenerationStore = staticGenerationAsyncStorage.getStore()
if (staticGenerationStore === undefined) {
throw new Error(
'"use cache" cannot be used outside of App Router. Expected a StaticGenerationStore.'
)
}

let stream
if (
entry === undefined ||
Expand All @@ -216,9 +224,16 @@ export function cache(kind: string, id: string, fn: any) {
// might include request specific things like cookies() inside a React.cache().
// Note: It is important that we await at least once before this because it lets us
// pop out of any stack specific contexts as well - aka "Sync" Local Storage.

// Get the clientReferenceManifestSingleton while we're still in the outer Context.
// In case getClientReferenceManifestSingleton is implemented using AsyncLocalStorage.
const clientReferenceManifestSingleton =
getClientReferenceManifestSingleton()

stream = await runInCleanSnapshot(
generateCacheEntry,
staticGenerationStore,
clientReferenceManifestSingleton,
cacheHandler,
serializedCacheKey,
encodedArguments,
Expand All @@ -229,9 +244,12 @@ export function cache(kind: string, id: string, fn: any) {
if (entry.stale) {
// If this is stale, and we're not in a prerender (i.e. this is dynamic render),
// then we should warm up the cache with a fresh revalidated entry.
const clientReferenceManifestSingleton =
getClientReferenceManifestSingleton()
const ignoredStream = await runInCleanSnapshot(
generateCacheEntry,
staticGenerationStore,
clientReferenceManifestSingleton,
cacheHandler,
serializedCacheKey,
encodedArguments,
Expand Down
1 change: 1 addition & 0 deletions packages/next/src/server/web/edge-route-module-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ export class EdgeRouteModuleWrapper {
after: isAfterEnabled,
dynamicIO: false,
},
buildId: '', // TODO: Populate this properly.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are not properly populated.

},
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ export function unstable_cache<T extends Callback>(
page: '/',
isStaticGeneration: false,
fallbackRouteParams: null,
buildId: '', // Since this is a fake one it can't "use cache" anyway.
},
cb,
...args
Expand Down
Loading