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

Fix revalidateTag with fetch and "use cache" #72075

Merged
merged 5 commits into from
Oct 30, 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
8 changes: 8 additions & 0 deletions packages/next/src/server/lib/patch-fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,14 @@ export function createPatchedFetcher(
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)
}
}
Comment on lines +274 to +280
Copy link
Contributor

@jonathanhefner jonathanhefner Oct 30, 2024

Choose a reason for hiding this comment

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

I was curious... is the end result of these lines + lines 266-272 to set both tags and workUnitStore.tags to their union? If so, would tags.splice(0, tags.length, ...collectedTags) work here instead?

Also, would Next.js welcome a PR with that kind of change? Or are such changes considered too cosmetic in the grand scale of performance?

}
}

Expand Down
7 changes: 0 additions & 7 deletions test/e2e/app-dir/use-cache/app/cache-tag/actions.ts

This file was deleted.

24 changes: 12 additions & 12 deletions test/e2e/app-dir/use-cache/app/cache-tag/button.tsx
Original file line number Diff line number Diff line change
@@ -1,36 +1,36 @@
'use client'
import { useRouter } from 'next/navigation'
import React from 'react'
import { revalidateWithTag } from './actions'
import { revalidateTag } from 'next/cache'

export function RevalidateButtons() {
const router = useRouter()
return (
<>
<form>
<button
id="revalidate-a"
onClick={() => {
revalidateWithTag('a').then(() => router.refresh())
formAction={async () => {
'use server'
revalidateTag('a')
}}
>
revalidate a
</button>
<button
id="revalidate-b"
onClick={() => {
revalidateWithTag('b').then(() => router.refresh())
formAction={async () => {
'use server'
revalidateTag('b')
}}
>
revalidate b
</button>
<button
id="revalidate-c"
onClick={() => {
revalidateWithTag('c').then(() => router.refresh())
formAction={async () => {
'use server'
revalidateTag('c')
}}
>
revalidate c
</button>
</>
</form>
)
}
14 changes: 10 additions & 4 deletions test/e2e/app-dir/use-cache/app/cache-tag/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,28 @@ import React from 'react'
import { unstable_cacheTag as cacheTag } from 'next/cache'
import { RevalidateButtons } from './button'

async function getCachedWithTag(tag) {
async function getCachedWithTag(tag: string) {
'use cache'
cacheTag(tag, 'c')
return Math.random()

const response = await fetch(
`https://next-data-api-endpoint.vercel.app/api/random?tag=${tag}`
)

return response.text()
}

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

return (
<p>
<div>
<p id="x">{x}</p>
<br />
<p id="y">{y}</p>
<br />
<RevalidateButtons />
</p>
</div>
)
}
12 changes: 7 additions & 5 deletions test/e2e/app-dir/use-cache/use-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ describe('use-cache', () => {
expect(rand1).toEqual(rand2)
})

it('should cache results for cached funtions imported from client components', async () => {
it('should cache results for cached functions imported from client components', async () => {
const browser = await next.browser('/imported-from-client')
expect(await browser.elementByCss('p').text()).toBe('0 0 0')
await browser.elementById('submit-button').click()
Expand All @@ -153,7 +153,7 @@ describe('use-cache', () => {
})
})

it('should cache results for cached funtions passed client components', async () => {
it('should cache results for cached functions passed to client components', async () => {
const browser = await next.browser('/passed-to-client')
expect(await browser.elementByCss('p').text()).toBe('0 0 0')
await browser.elementById('submit-button').click()
Expand Down Expand Up @@ -182,19 +182,21 @@ describe('use-cache', () => {

const initialX = await browser.elementByCss('#x').text()
const initialY = await browser.elementByCss('#y').text()
let updatedX
let updatedY
let updatedX: string | undefined
let updatedY: string | undefined

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)
})

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)
})

await browser.elementByCss('#revalidate-c').click()
Expand Down Expand Up @@ -242,7 +244,7 @@ describe('use-cache', () => {
})
})

it('should be able to revalidate a page using', async () => {
it('should be able to revalidate a page using revalidateTag', async () => {
const browser = await next.browser(`/form`)
const time1 = await browser.waitForElementByCss('#t').text()

Expand Down
Loading