Skip to content

Commit 325dc4b

Browse files
committedAug 26, 2024
pages router: ensure x-middleware-cache is respected (#67734)
`x-middleware-cache: "no-store"` in Pages router is a way to signal to the client that it should not store the response in the cache. However in certain circumstances, namely when `unstable_skipClientCache` is true, the data request would be awaited and then stored in the `inflightCache` regardless of the header. The original implementation of this in the router has logic to delete the response from the inflight cache after the request has fulfilled because `inflightCache` stores the unresolved promise. But in this optimistic prefetch case, when we're only storing it in the cache once the request is fulfilled, we can prevent a race condition where the ignored prefetch is erroneously re-added to the cache by ensuring it's never added to the cache to begin with if the response says not to. Fixes #66881 Closes NEXT-3550

File tree

5 files changed

+72
-2
lines changed

5 files changed

+72
-2
lines changed
 

‎packages/next/src/shared/lib/router/router.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,11 @@ function fetchNextData({
588588
// without blocking navigation when stale data is available
589589
if (unstable_skipClientCache && persistCache) {
590590
return getData({}).then((data) => {
591-
inflightCache[cacheKey] = Promise.resolve(data)
591+
if (data.response.headers.get('x-middleware-cache') !== 'no-cache') {
592+
// only update cache if not marked as no-cache
593+
inflightCache[cacheKey] = Promise.resolve(data)
594+
}
595+
592596
return data
593597
})
594598
}

‎test/e2e/middleware-rewrites/app/middleware.js

+13
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,19 @@ export async function middleware(request) {
109109
return NextResponse.rewrite(url)
110110
}
111111

112+
if (url.pathname === '/dynamic-no-cache/1') {
113+
const rewriteUrl =
114+
request.headers.get('purpose') === 'prefetch'
115+
? '/dynamic-no-cache/1'
116+
: '/dynamic-no-cache/2'
117+
118+
url.pathname = rewriteUrl
119+
120+
return NextResponse.rewrite(url, {
121+
headers: { 'x-middleware-cache': 'no-cache' },
122+
})
123+
}
124+
112125
if (
113126
url.pathname === '/rewrite-me-without-hard-navigation' ||
114127
url.searchParams.get('path') === 'rewrite-me-without-hard-navigation'
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
export default function Dynamic({ id }) {
2+
return (
3+
<main id="dynamic-page">
4+
<h1>Page {id}</h1>
5+
</main>
6+
)
7+
}
8+
9+
export const getStaticProps = async ({ params }) => {
10+
return {
11+
props: {
12+
id: params.id,
13+
},
14+
}
15+
}
16+
17+
export const getStaticPaths = async () => {
18+
return {
19+
paths: [{ params: { id: '1' } }, { params: { id: '2' } }],
20+
fallback: false,
21+
}
22+
}

‎test/e2e/middleware-rewrites/app/pages/index.js

+1
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ export default function Home() {
6666
<Link href="/ssg" id="normal-ssg-link">
6767
normal SSG link
6868
</Link>
69+
<Link href="/dynamic-no-cache/1">/dynamic-no-cache/1</Link>
6970
<div />
7071
<a
7172
href=""

‎test/e2e/middleware-rewrites/test/index.test.ts

+31-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { NextInstance } from 'e2e-utils'
77
import { check, fetchViaHTTP, retry } from 'next-test-utils'
88
import { createNext, FileRef } from 'e2e-utils'
99
import escapeStringRegexp from 'escape-string-regexp'
10-
import { Request } from 'playwright'
10+
import { Request, Response } from 'playwright'
1111

1212
describe('Middleware Rewrite', () => {
1313
let next: NextInstance
@@ -363,6 +363,36 @@ describe('Middleware Rewrite', () => {
363363
})
364364

365365
if (!(global as any).isNextDev) {
366+
it('should opt out of prefetch caching for dynamic routes', async () => {
367+
const browser = await webdriver(next.url, '/')
368+
await browser.eval('window.__SAME_PAGE = true')
369+
await browser.waitForIdleNetwork()
370+
let hasResolvedPrefetch = false
371+
372+
browser.on('response', async (res: Response) => {
373+
const req = res.request()
374+
const headers = await req.allHeaders()
375+
if (
376+
headers['purpose'] === 'prefetch' &&
377+
req.url().includes('/dynamic-no-cache/1')
378+
) {
379+
hasResolvedPrefetch = true
380+
}
381+
})
382+
383+
await browser.elementByCss("[href='/dynamic-no-cache/1']").moveTo()
384+
385+
await retry(async () => {
386+
expect(hasResolvedPrefetch).toBe(true)
387+
})
388+
389+
await browser.elementByCss("[href='/dynamic-no-cache/1']").click()
390+
391+
await browser.waitForElementByCss('#dynamic-page')
392+
expect(await browser.elementById('dynamic-page').text()).toBe('Page 2')
393+
expect(await browser.eval('window.__SAME_PAGE')).toBe(true)
394+
})
395+
366396
it('should not prefetch non-SSG routes', async () => {
367397
const browser = await webdriver(next.url, '/')
368398

0 commit comments

Comments
 (0)
Please sign in to comment.