Skip to content

Commit

Permalink
Do not cache fetch calls inside "use cache" implicitly
Browse files Browse the repository at this point in the history
If a `fetch` call inside of `"use cache"` has not specified `cache:
'force-cache'` or `revalidate`, we should not cache it implicitly in the
fetch cache. Instead, it's only cached as part of the surrounding cached
function.

Those `fetch` calls that do specify `cache: 'force-cache'` or
`revalidate` are still cached in the fetch cache. They're like an inner
`"use cache"`. They also don't inherit the tags of the outer `"use
cache"`.

partially reverts #71793 and #72075
supersedes #72090
  • Loading branch information
unstubbable committed Oct 30, 2024
1 parent c0052b8 commit 375e642
Show file tree
Hide file tree
Showing 5 changed files with 181 additions and 84 deletions.
38 changes: 12 additions & 26 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,12 +281,10 @@ export function createPatchedFetcher(
? []
: workUnitStore.implicitTags

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

Expand Down Expand Up @@ -359,15 +352,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 +499,9 @@ export function createPatchedFetcher(
}
}

if (revalidateStore) {
// We only want to set the revalidate store's revalidate time if it
// was explicitly set for the fetch call, i.e. currentFetchRevalidate.
if (revalidateStore && currentFetchRevalidate === finalRevalidate) {
revalidateStore.revalidate = finalRevalidate
}
}
Expand Down
36 changes: 0 additions & 36 deletions test/e2e/app-dir/use-cache/app/cache-tag/button.tsx

This file was deleted.

63 changes: 63 additions & 0 deletions test/e2e/app-dir/use-cache/app/cache-tag/buttons.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import React from 'react'
import { revalidatePath, revalidateTag } from 'next/cache'

export function RevalidateButtons() {
return (
<form>
<button
id="revalidate-a"
formAction={async () => {
'use server'
revalidateTag('a')
}}
>
revalidate a
</button>{' '}
<button
id="revalidate-b"
formAction={async () => {
'use server'
revalidateTag('b')
}}
>
revalidate b
</button>{' '}
<button
id="revalidate-c"
formAction={async () => {
'use server'
revalidateTag('c')
}}
>
revalidate c
</button>{' '}
<button
id="revalidate-f"
formAction={async () => {
'use server'
revalidateTag('f')
}}
>
revalidate f
</button>{' '}
<button
id="revalidate-r"
formAction={async () => {
'use server'
revalidateTag('r')
}}
>
revalidate r
</button>{' '}
<button
id="revalidate-path"
formAction={async () => {
'use server'
revalidatePath('/cache-tag')
}}
>
revalidate path
</button>
</form>
)
}
49 changes: 39 additions & 10 deletions test/e2e/app-dir/use-cache/app/cache-tag/page.tsx
Original file line number Diff line number Diff line change
@@ -1,28 +1,57 @@
import React from 'react'
import { unstable_cacheTag as cacheTag } from 'next/cache'
import { RevalidateButtons } from './button'
import { RevalidateButtons } from './buttons'
import { connection } from 'next/server'

async function getCachedWithTag(tag: string) {
async function getCachedWithTag({
tag,
fetchCache,
}: {
tag: string
fetchCache?: 'force' | 'revalidate'
}) {
'use cache'
cacheTag(tag, 'c')

// If forceCache is true, revalidating tag 'c' won't revalidate the fetch
// cache.
const response = await fetch(
`https://next-data-api-endpoint.vercel.app/api/random?tag=${tag}`
`https://next-data-api-endpoint.vercel.app/api/random?tag=${tag}`,
{
cache: fetchCache === 'force' ? 'force-cache' : undefined,
next: { revalidate: fetchCache === 'revalidate' ? 42 : undefined },
}
)

return response.text()
const fetchedValue = await response.text()

return [Math.random(), fetchedValue]
}

export default async function Page() {
const x = await getCachedWithTag('a')
const y = await getCachedWithTag('b')
await connection()

const a = await getCachedWithTag({ tag: 'a' })
const b = await getCachedWithTag({ tag: 'b' })

const [f1, f2] = await getCachedWithTag({
tag: 'f',
fetchCache: 'force',
})

const [r1, r2] = await getCachedWithTag({
tag: 'r',
fetchCache: 'revalidate',
})

return (
<div>
<p id="x">{x}</p>
<br />
<p id="y">{y}</p>
<br />
<p id="a">[a, c] {a.join(' ')}</p>
<p id="b">[b, c] {b.join(' ')}</p>
<p id="f1">[f, c] {f1}</p>
<p id="f2">[-] {f2}</p>
<p id="r1">[r, c] {r1}</p>
<p id="r2">[-] {r2}</p>
<RevalidateButtons />
</div>
)
Expand Down
79 changes: 67 additions & 12 deletions test/e2e/app-dir/use-cache/use-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,29 +180,84 @@ describe('use-cache', () => {
it('should update after revalidateTag correctly', async () => {
const browser = await next.browser('/cache-tag')

const initialX = await browser.elementByCss('#x').text()
const initialY = await browser.elementByCss('#y').text()
let updatedX: string | undefined
let updatedY: string | undefined
let valueA = await browser.elementByCss('#a').text()
let valueB = await browser.elementByCss('#b').text()
let valueF1 = await browser.elementByCss('#f1').text()
let valueF2 = await browser.elementByCss('#f2').text()
let valueR1 = await browser.elementByCss('#r1').text()
let valueR2 = await browser.elementByCss('#r2').text()

await browser.elementByCss('#revalidate-a').click()
await retry(async () => {
updatedX = await browser.elementByCss('#x').text()
expect(updatedX).not.toBe(initialX)
expect(await browser.elementByCss('#y').text()).toBe(initialY)
expect(await browser.elementByCss('#a').text()).not.toBe(valueA)
expect(await browser.elementByCss('#b').text()).toBe(valueB)
expect(await browser.elementByCss('#f1').text()).toBe(valueF1)
expect(await browser.elementByCss('#f2').text()).toBe(valueF2)
expect(await browser.elementByCss('#r1').text()).toBe(valueR1)
expect(await browser.elementByCss('#r2').text()).toBe(valueR2)
})

valueA = await browser.elementByCss('#a').text()

await browser.elementByCss('#revalidate-b').click()
await retry(async () => {
updatedY = await browser.elementByCss('#y').text()
expect(updatedY).not.toBe(initialY)
expect(await browser.elementByCss('#x').text()).toBe(updatedX)
expect(await browser.elementByCss('#a').text()).toBe(valueA)
expect(await browser.elementByCss('#b').text()).not.toBe(valueB)
expect(await browser.elementByCss('#f1').text()).toBe(valueF1)
expect(await browser.elementByCss('#f2').text()).toBe(valueF2)
expect(await browser.elementByCss('#r1').text()).toBe(valueR1)
expect(await browser.elementByCss('#r2').text()).toBe(valueR2)
})

valueB = await browser.elementByCss('#b').text()

await browser.elementByCss('#revalidate-c').click()
await retry(async () => {
expect(await browser.elementByCss('#x').text()).not.toBe(updatedX)
expect(await browser.elementByCss('#y').text()).not.toBe(updatedY)
expect(await browser.elementByCss('#a').text()).not.toBe(valueA)
expect(await browser.elementByCss('#b').text()).not.toBe(valueB)
expect(await browser.elementByCss('#f1').text()).not.toBe(valueF1)
expect(await browser.elementByCss('#f2').text()).toBe(valueF2)
expect(await browser.elementByCss('#r1').text()).not.toBe(valueR1)
expect(await browser.elementByCss('#r2').text()).toBe(valueR2)
})

valueA = await browser.elementByCss('#a').text()
valueB = await browser.elementByCss('#b').text()
valueF1 = await browser.elementByCss('#f1').text()
valueR1 = await browser.elementByCss('#r1').text()

await browser.elementByCss('#revalidate-f').click()
await retry(async () => {
expect(await browser.elementByCss('#a').text()).toBe(valueA)
expect(await browser.elementByCss('#b').text()).toBe(valueB)
expect(await browser.elementByCss('#f1').text()).not.toBe(valueF1)
expect(await browser.elementByCss('#f2').text()).toBe(valueF2)
expect(await browser.elementByCss('#r1').text()).toBe(valueR1)
expect(await browser.elementByCss('#r2').text()).toBe(valueR2)
})

valueF1 = await browser.elementByCss('#f1').text()

await browser.elementByCss('#revalidate-r').click()
await retry(async () => {
expect(await browser.elementByCss('#a').text()).toBe(valueA)
expect(await browser.elementByCss('#b').text()).toBe(valueB)
expect(await browser.elementByCss('#f1').text()).toBe(valueF1)
expect(await browser.elementByCss('#f2').text()).toBe(valueF2)
expect(await browser.elementByCss('#r1').text()).not.toBe(valueR1)
expect(await browser.elementByCss('#r2').text()).toBe(valueR2)
})

valueR1 = await browser.elementByCss('#r1').text()

await browser.elementByCss('#revalidate-path').click()
await retry(async () => {
expect(await browser.elementByCss('#a').text()).not.toBe(valueA)
expect(await browser.elementByCss('#b').text()).not.toBe(valueB)
expect(await browser.elementByCss('#f1').text()).not.toBe(valueF1)
expect(await browser.elementByCss('#f2').text()).not.toBe(valueF2)
expect(await browser.elementByCss('#r1').text()).not.toBe(valueR1)
expect(await browser.elementByCss('#r2').text()).not.toBe(valueR2)
})
})
}
Expand Down

0 comments on commit 375e642

Please sign in to comment.