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.

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

Do not cache fetch calls in "use cache" #72090

Closed
wants to merge 1 commit into from
Closed
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
72 changes: 30 additions & 42 deletions packages/next/src/server/lib/patch-fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,30 +254,25 @@ export function createPatchedFetcher(
`fetch ${input.toString()}`
)

if (
const revalidateStore =
workUnitStore &&
(workUnitStore.type === 'cache' ||
workUnitStore.type === 'prerender' ||
workUnitStore.type === 'prerender-ppr' ||
workUnitStore.type === 'prerender-legacy')
) {
? workUnitStore
: undefined

if (revalidateStore) {
if (Array.isArray(tags)) {
// Collect tags onto parent caches or parent prerenders.
const collectedTags =
workUnitStore.tags ?? (workUnitStore.tags = [])
revalidateStore.tags ?? (revalidateStore.tags = [])
for (const tag of tags) {
if (!collectedTags.includes(tag)) {
collectedTags.push(tag)
}
}

// Add tags of the current cache scope to the locally collected tags
// for this fetch call.
for (const tag of collectedTags) {
if (!tags.includes(tag)) {
tags.push(tag)
}
}
}
}

Expand All @@ -286,14 +281,16 @@ export function createPatchedFetcher(
? []
: workUnitStore.implicitTags

const isInCacheScope = workUnitStore
? workUnitStore.type === 'unstable-cache' ||
workUnitStore.type === 'cache'
: false

// Inside unstable-cache or "use cache", we treat it the same as
// force-no-store on the page.
const pageFetchCacheMode =
workUnitStore &&
(workUnitStore.type === 'unstable-cache' ||
workUnitStore.type === 'cache')
? 'force-no-store'
: workStore.fetchCache
const pageFetchCacheMode = isInCacheScope
? 'force-no-store'
: workStore.fetchCache

const isUsingNoStore = !!workStore.isUnstableNoStore

Expand All @@ -313,23 +310,23 @@ export function createPatchedFetcher(
currentFetchCacheConfig = undefined
}

if (currentFetchCacheConfig === 'force-cache') {
if (isInCacheScope) {
// TODO: Warn if `cache` or `revalidate configs have been set?
currentFetchCacheConfig = undefined
currentFetchRevalidate = 0
} else if (currentFetchCacheConfig === 'force-cache') {
currentFetchRevalidate = false
} else if (
// if we are inside of "use cache"/"unstable_cache"
// we shouldn't set the revalidate to 0 as it's overridden
// by the cache context
workUnitStore?.type !== 'cache' &&
(currentFetchCacheConfig === 'no-cache' ||
currentFetchCacheConfig === 'no-store' ||
pageFetchCacheMode === 'force-no-store' ||
pageFetchCacheMode === 'only-no-store' ||
// If no explicit fetch cache mode is set, but dynamic = `force-dynamic` is set,
// we shouldn't consider caching the fetch. This is because the `dynamic` cache
// is considered a "top-level" cache mode, whereas something like `fetchCache` is more
// fine-grained. Top-level modes are responsible for setting reasonable defaults for the
// other configurations.
(!pageFetchCacheMode && workStore.forceDynamic))
currentFetchCacheConfig === 'no-cache' ||
currentFetchCacheConfig === 'no-store' ||
pageFetchCacheMode === 'force-no-store' ||
pageFetchCacheMode === 'only-no-store' ||
// If no explicit fetch cache mode is set, but dynamic = `force-dynamic` is set,
// we shouldn't consider caching the fetch. This is because the `dynamic` cache
// is considered a "top-level" cache mode, whereas something like `fetchCache` is more
// fine-grained. Top-level modes are responsible for setting reasonable defaults for the
// other configurations.
(!pageFetchCacheMode && workStore.forceDynamic)
) {
currentFetchRevalidate = 0
}
Expand Down Expand Up @@ -359,15 +356,6 @@ export function createPatchedFetcher(
getRequestMeta('method')?.toLowerCase() || 'get'
)

const revalidateStore =
workUnitStore &&
(workUnitStore.type === 'cache' ||
workUnitStore.type === 'prerender' ||
workUnitStore.type === 'prerender-ppr' ||
workUnitStore.type === 'prerender-legacy')
? workUnitStore
: undefined

/**
* We automatically disable fetch caching under the following conditions:
* - Fetch cache configs are not set. Specifically:
Expand Down Expand Up @@ -515,7 +503,7 @@ export function createPatchedFetcher(
}
}

if (revalidateStore) {
if (revalidateStore && !isInCacheScope) {
revalidateStore.revalidate = finalRevalidate
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/app-dir/use-cache/use-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,13 +269,13 @@ describe('use-cache', () => {
// expect(time4).toBe(time3);
})

it('should use revalidate config in fetch', async () => {
it('should ignore revalidate config in fetch', async () => {
const browser = await next.browser('/fetch-revalidate')

const initialValue = await browser.elementByCss('#random').text()
await browser.refresh()

expect(await browser.elementByCss('#random').text()).not.toBe(initialValue)
expect(await browser.elementByCss('#random').text()).toBe(initialValue)
})

it('should cache fetch without no-store', async () => {
Expand Down
Loading