Skip to content

Commit

Permalink
fix(fetch-cache): add check for updated tags when checking same cache…
Browse files Browse the repository at this point in the history
… key (#63547)

## Why?

When we fetch the same cache key (URL) but add an additional tag, the
revalidation does not re-fetch correctly (the bug just uses in-memory
cache again) when deployed.

:repro: →
https://github.com/lostip/nextjs-revalidation-demo/tree/main

---------

Co-authored-by: Ethan Arrowood <ethan@arrowood.dev>
  • Loading branch information
samcx and Ethan-Arrowood authored Apr 9, 2024
1 parent 8c9c1d2 commit 39a1c2a
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 2 deletions.
34 changes: 32 additions & 2 deletions packages/next/src/server/lib/incremental-cache/fetch-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,21 @@ export default class FetchCache implements CacheHandler {
private cacheEndpoint?: string
private debug: boolean

private hasMatchingTags(arr1: string[], arr2: string[]) {
if (arr1.length !== arr2.length) return false

const set1 = new Set(arr1)
const set2 = new Set(arr2)

if (set1.size !== set2.size) return false

for (let tag of set1) {
if (!set2.has(tag)) return false
}

return true
}

static isAvailable(ctx: {
_requestHeaders: CacheHandlerContext['_requestHeaders']
}) {
Expand Down Expand Up @@ -168,8 +183,13 @@ export default class FetchCache implements CacheHandler {
// on successive requests
let data = memoryCache?.get(key)

// get data from fetch cache
if (!data && this.cacheEndpoint) {
const hasFetchKindAndMatchingTags =
data?.value?.kind === 'FETCH' &&
this.hasMatchingTags(tags ?? [], data.value.tags ?? [])

// Get data from fetch cache. Also check if new tags have been
// specified with the same cache key (fetch URL)
if (this.cacheEndpoint && (!data || !hasFetchKindAndMatchingTags)) {
try {
const start = Date.now()
const fetchParams: NextFetchCacheParams = {
Expand Down Expand Up @@ -220,6 +240,16 @@ export default class FetchCache implements CacheHandler {
throw new Error(`invalid cache value`)
}

// if new tags were specified, merge those tags to the existing tags
if (cached.kind === 'FETCH') {
cached.tags ??= []
for (const tag of tags ?? []) {
if (!cached.tags.include(tag)) {
cached.tag.push(tag)
}
}
}

const cacheState = res.headers.get(CACHE_STATE_HEADER)
const age = res.headers.get('age')

Expand Down
47 changes: 47 additions & 0 deletions test/e2e/app-dir/app-static/app-static.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,49 @@ createNextDescribe(
})
})

if (isNextDeploy) {
describe('new tags have been specified on subsequent fetch', () => {
it('should not fetch from memory cache', async () => {
const res1 = await next.fetch('/specify-new-tags/one-tag')
expect(res1.status).toBe(200)

const res2 = await next.fetch('/specify-new-tags/two-tags')
expect(res2.status).toBe(200)

const html1 = await res1.text()
const html2 = await res2.text()
const $1 = cheerio.load(html1)
const $2 = cheerio.load(html2)

const data1 = $1('#page-data').text()
const data2 = $2('#page-data').text()
expect(data1).not.toBe(data2)
})

it('should not fetch from memory cache after revalidateTag is used', async () => {
const res1 = await next.fetch('/specify-new-tags/one-tag')
expect(res1.status).toBe(200)

const revalidateRes = await next.fetch(
'/api/revlidate-tag-node?tag=thankyounext'
)
expect((await revalidateRes.json()).revalidated).toBe(true)

const res2 = await next.fetch('/specify-new-tags/two-tags')
expect(res2.status).toBe(200)

const html1 = await res1.text()
const html2 = await res2.text()
const $1 = cheerio.load(html1)
const $2 = cheerio.load(html2)

const data1 = $1('#page-data').text()
const data2 = $2('#page-data').text()
expect(data1).not.toBe(data2)
})
})
}

if (isNextStart) {
it('should propagate unstable_cache tags correctly', async () => {
const meta = JSON.parse(
Expand Down Expand Up @@ -717,6 +760,10 @@ createNextDescribe(
"route-handler/revalidate-360-isr/route.js",
"route-handler/revalidate-360/route.js",
"route-handler/static-cookies/route.js",
"specify-new-tags/one-tag/page.js",
"specify-new-tags/one-tag/page_client-reference-manifest.js",
"specify-new-tags/two-tags/page.js",
"specify-new-tags/two-tags/page_client-reference-manifest.js",
"ssg-draft-mode.html",
"ssg-draft-mode.rsc",
"ssg-draft-mode/[[...route]]/page.js",
Expand Down
13 changes: 13 additions & 0 deletions test/e2e/app-dir/app-static/app/specify-new-tags/one-tag/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export const dynamic = 'force-dynamic'

export default async function Page() {
const data = await fetch(
'https://next-data-api-endpoint.vercel.app/api/random?sam=iam',
{
cache: 'force-cache',
next: { tags: ['thankyounext'] },
}
).then((res) => res.text())

return <p id="page-data">data: {data}</p>
}
13 changes: 13 additions & 0 deletions test/e2e/app-dir/app-static/app/specify-new-tags/two-tags/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export const dynamic = 'force-dynamic'

export default async function Page() {
const data = await fetch(
'https://next-data-api-endpoint.vercel.app/api/random?sam=iam',
{
cache: 'force-cache',
next: { tags: ['thankyounext', 'justputit'] },
}
).then((res) => res.text())

return <p id="page-data">data: {data}</p>
}

0 comments on commit 39a1c2a

Please sign in to comment.