Skip to content
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.

Sign up for GitHub

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

Remove shouldRevalidateStale concept from CacheHandlers #70493

Merged
merged 4 commits into from
Sep 26, 2024
Merged

Remove shouldRevalidateStale concept from CacheHandlers #70493

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6dc1336
Remove shouldRevalidateStale concept from CacheHandlers
sebmarkbage Sep 26, 2024
50ae05b
Merge branch 'canary' into cacheexpires
sebmarkbage Sep 26, 2024
781eda1
Merge branch 'canary' into cacheexpires
sebmarkbage Sep 26, 2024
a165487
Merge branch 'canary' into cacheexpires
sebmarkbage Sep 26, 2024
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
18 changes: 8 additions & 10 deletions packages/next/src/server/use-cache/use-cache-wrapper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,17 @@ import type { ManifestNode } from '../../build/webpack/plugins/flight-manifest-p

type CacheEntry = {
value: ReadableStream
// In-memory caches are fragile and should not use stale-while-revalidate
// semantics on the caches because it's not worth warming up an entry that's
// likely going to get evicted before we get to use it anyway. However,
// we also don't want to reuse a stale entry for too long so stale entries
// should be considered expired/missing in such CacheHandlers.
stale: boolean
}

interface CacheHandler {
get(cacheKey: string | ArrayBuffer): Promise<undefined | CacheEntry>
set(cacheKey: string | ArrayBuffer, value: ReadableStream): Promise<void>
shouldRevalidateStale: boolean
}

const cacheHandlerMap: Map<string, CacheHandler> = new Map()
Expand Down Expand Up @@ -64,10 +68,6 @@ cacheHandlerMap.set('default', {
await value.cancel()
}
},
// In-memory caches are fragile and should not use stale-while-revalidate
// semantics on the caches because it's not worth warming up an entry that's
// likely going to get evicted before we get to use it anyway.
shouldRevalidateStale: false,
})

// TODO: Consider moving this another module that is guaranteed to be required in a safe scope.
Expand Down Expand Up @@ -198,7 +198,7 @@ export function cache(kind: string, id: string, fn: any) {
let stream
if (
entry === undefined ||
(staticGenerationStore.isStaticGeneration && entry.stale)
(entry.stale && staticGenerationStore.isStaticGeneration)
) {
// Miss. Generate a new result.

Expand All @@ -221,11 +221,9 @@ export function cache(kind: string, id: string, fn: any) {
)
} else {
stream = entry.value
if (entry.stale && cacheHandler.shouldRevalidateStale) {
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. We only do this
// for long lived cache handlers because it's not worth warming up the cache with an
// an entry that's just going to get evicted before we can use it anyway.
// then we should warm up the cache with a fresh revalidated entry.
const ignoredStream = await runInCleanSnapshot(
generateCacheEntry,
staticGenerationStore,
Expand Down
Loading