fix router prefetch cache key to work with route interception (#59861)

### What?
When handling route interception in two different segments but handled
by the same interception route, the first interception will show the
correct component but attempting the same interception on another
segment will return elements from the first request, not the second.

### Why?
Prefetch cache entries are created from the browser URL. However, route
interception makes use of `nextUrl` to mask the underlying components
that are being fetched from the server to handle the request

Consider the following scenario:

```
app
   foo
     @modal
       (...)post
         [id]
   bar
     @modal
       (...post)
         [id]
   post
     [id]
     
```
If you trigger an interception on `/foo`, your URL is going to be masked
to `/post/1` and keyed as such in the prefetch cache. However, the cache
entry is actually associated with `app/foo/@modal/(...post)/[id]`. That
means when you trigger the same interception on `/bar`, it will return
the tree from `/foo`.

### How?
This PR will prefix the prefetch cache key with `state.nextUrl` when
necessary.

Fixes #49878
Fixes #52748
Closes NEXT-1818
This commit is contained in:
Zack Tanner 2023-12-22 13:10:37 -08:00 committed by GitHub
parent c4adae89b1
commit 6545783be3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 239 additions and 13 deletions

View file

@ -0,0 +1,29 @@
import { addPathPrefix } from '../../../../shared/lib/router/utils/add-path-prefix'
import { pathHasPrefix } from '../../../../shared/lib/router/utils/path-has-prefix'
import { createHrefFromUrl } from '../create-href-from-url'
/**
* Creates a cache key for the router prefetch cache
*
* @param url - The URL being navigated to
* @param nextUrl - an internal URL, primarily used for handling rewrites. Defaults to '/'.
* @return The generated prefetch cache key.
*/
export function createPrefetchCacheKey(url: URL, nextUrl: string | null) {
const pathnameFromUrl = createHrefFromUrl(
url,
// Ensures the hash is not part of the cache key as it does not impact the server fetch
false
)
// delimit the prefix so we don't conflict with other pages
const nextUrlPrefix = `${nextUrl}%`
// Route interception depends on `nextUrl` values which aren't a 1:1 mapping to a URL
// The cache key that we store needs to use `nextUrl` to properly distinguish cache entries
if (nextUrl && !pathHasPrefix(pathnameFromUrl, nextUrl)) {
return addPathPrefix(pathnameFromUrl, nextUrlPrefix)
}
return pathnameFromUrl
}

View file

@ -32,6 +32,7 @@ import {
listenForDynamicRequest,
updateCacheNodeOnNavigation,
} from '../ppr-navigations'
import { createPrefetchCacheKey } from './create-prefetch-cache-key'
export function handleExternalUrl(
state: ReadonlyReducerState,
@ -126,7 +127,8 @@ function navigateReducer_noPPR(
return handleExternalUrl(state, mutable, url.toString(), pendingPush)
}
let prefetchValues = state.prefetchCache.get(createHrefFromUrl(url, false))
const prefetchCacheKey = createPrefetchCacheKey(url, state.nextUrl)
let prefetchValues = state.prefetchCache.get(prefetchCacheKey)
// If we don't have a prefetch value, we need to create one
if (!prefetchValues) {
@ -152,7 +154,7 @@ function navigateReducer_noPPR(
lastUsedTime: null,
}
state.prefetchCache.set(createHrefFromUrl(url, false), newPrefetchValue)
state.prefetchCache.set(prefetchCacheKey, newPrefetchValue)
prefetchValues = newPrefetchValue
}
@ -320,7 +322,8 @@ function navigateReducer_PPR(
return handleExternalUrl(state, mutable, url.toString(), pendingPush)
}
let prefetchValues = state.prefetchCache.get(createHrefFromUrl(url, false))
const prefetchCacheKey = createPrefetchCacheKey(url, state.nextUrl)
let prefetchValues = state.prefetchCache.get(prefetchCacheKey)
// If we don't have a prefetch value, we need to create one
if (!prefetchValues) {
@ -346,7 +349,7 @@ function navigateReducer_PPR(
lastUsedTime: null,
}
state.prefetchCache.set(createHrefFromUrl(url, false), newPrefetchValue)
state.prefetchCache.set(prefetchCacheKey, newPrefetchValue)
prefetchValues = newPrefetchValue
}

View file

@ -1,4 +1,3 @@
import { createHrefFromUrl } from '../create-href-from-url'
import { fetchServerResponse } from '../fetch-server-response'
import type {
PrefetchAction,
@ -9,6 +8,7 @@ import { PrefetchKind } from '../router-reducer-types'
import { prunePrefetchCache } from './prune-prefetch-cache'
import { NEXT_RSC_UNION_QUERY } from '../../app-router-headers'
import { PromiseQueue } from '../../promise-queue'
import { createPrefetchCacheKey } from './create-prefetch-cache-key'
export const prefetchQueue = new PromiseQueue(5)
@ -22,20 +22,16 @@ export function prefetchReducer(
const { url } = action
url.searchParams.delete(NEXT_RSC_UNION_QUERY)
const href = createHrefFromUrl(
url,
// Ensures the hash is not part of the cache key as it does not affect fetching the server
false
)
const prefetchCacheKey = createPrefetchCacheKey(url, state.nextUrl)
const cacheEntry = state.prefetchCache.get(prefetchCacheKey)
const cacheEntry = state.prefetchCache.get(href)
if (cacheEntry) {
/**
* If the cache entry present was marked as temporary, it means that we prefetched it from the navigate reducer,
* where we didn't have the prefetch intent. We want to update it to the new, more accurate, kind here.
*/
if (cacheEntry.kind === PrefetchKind.TEMPORARY) {
state.prefetchCache.set(href, {
state.prefetchCache.set(prefetchCacheKey, {
...cacheEntry,
kind: action.kind,
})
@ -68,7 +64,7 @@ export function prefetchReducer(
)
// Create new tree based on the flightSegmentPath and router state patch
state.prefetchCache.set(href, {
state.prefetchCache.set(prefetchCacheKey, {
// Create new tree based on the flightSegmentPath and router state patch
treeAtTimeOfPrefetch: state.tree,
data: serverResponse,

View file

@ -0,0 +1,27 @@
'use client'
import { useRouter } from 'next/navigation'
interface ModalProps {
title: string
context: string
}
export function Modal({ title, context }: ModalProps) {
const router = useRouter()
return (
<div>
<div className="modal">
<h1>{title}</h1>
<h2>{context}</h2>
</div>
<div
className="modal-overlay"
onClick={() => {
router.back()
}}
/>
</div>
)
}

View file

@ -0,0 +1,11 @@
import { Modal } from '../../../../Modal'
export default function BarPagePostInterceptSlot({
params: { id },
}: {
params: {
id: string
}
}) {
return <Modal title={`Post ${id}`} context="Intercepted on Bar Page" />
}

View file

@ -0,0 +1,3 @@
export default function Default() {
return null
}

View file

@ -0,0 +1,3 @@
export default function Default() {
return null
}

View file

@ -0,0 +1,14 @@
export default function BarLayout({
modal,
children,
}: {
modal: React.ReactNode
children: React.ReactNode
}) {
return (
<>
<div id="slot">{modal}</div>
<div id="children">{children}</div>
</>
)
}

View file

@ -0,0 +1,10 @@
import Link from 'next/link'
export default function BarPage() {
return (
<div>
<h1>Bar Page</h1>
<Link href="/post/1">Post 1</Link>
</div>
)
}

View file

@ -0,0 +1,11 @@
import { Modal } from '../../../../Modal'
export default function FooPagePostInterceptSlot({
params: { id },
}: {
params: {
id: string
}
}) {
return <Modal title={`Post ${id}`} context="Intercepted on Foo Page" />
}

View file

@ -0,0 +1,3 @@
export default function Default() {
return null
}

View file

@ -0,0 +1,3 @@
export default function Default() {
return null
}

View file

@ -0,0 +1,14 @@
export default function FooLayout({
modal,
children,
}: {
modal: React.ReactNode
children: React.ReactNode
}) {
return (
<>
<div id="slot">{modal}</div>
<div id="children">{children}</div>
</>
)
}

View file

@ -0,0 +1,10 @@
import Link from 'next/link'
export default function FooPage() {
return (
<div>
<h1>Foo Page</h1>
<Link href="/post/1">Post 1</Link>
</div>
)
}

View file

@ -0,0 +1,13 @@
import Link from 'next/link'
export default function RootLayout({ children }) {
return (
<html>
<head />
<body>
<Link href="/">home</Link>
{children}
</body>
</html>
)
}

View file

@ -0,0 +1,18 @@
import Link from 'next/link'
export default function Home() {
return (
<ul>
<li>
<Link href="/foo">foo</Link>
</li>
<li>
<Link href="/bar">bar</Link>
</li>
<br />
<li>
<Link href="/post/1">post 1</Link>
</li>
</ul>
)
}

View file

@ -0,0 +1,13 @@
export default function PostPage({
params: { id },
}: {
params: {
id: string
}
}) {
return (
<div>
<h1>Post {id}</h1>
</div>
)
}

View file

@ -0,0 +1,45 @@
import { createNextDescribe } from 'e2e-utils'
import { check } from 'next-test-utils'
createNextDescribe(
'interception-route-prefetch-cache',
{
files: __dirname,
},
({ next }) => {
it('should render the correct interception when two distinct layouts share the same path structure', async () => {
const browser = await next.browser('/')
await browser.elementByCss('[href="/foo"]').click()
await check(() => browser.elementById('children').text(), /Foo Page/)
await browser.elementByCss('[href="/post/1"]').click()
// Ensure the existing page content is still the same
await check(() => browser.elementById('children').text(), /Foo Page/)
// Verify we got the right interception
await check(
() => browser.elementById('slot').text(),
/Intercepted on Foo Page/
)
// Go back home. At this point, the router cache should have content from /foo
// Now we want to ensure that /bar doesn't share that same prefetch cache entry
await browser.elementByCss('[href="/"]').click()
await browser.elementByCss('[href="/bar"]').click()
await check(() => browser.elementById('children').text(), /Bar Page/)
await browser.elementByCss('[href="/post/1"]').click()
// Ensure the existing page content is still the same. If the prefetch cache resolved the wrong cache node
// then we'd see the content from /foo
await check(() => browser.elementById('children').text(), /Bar Page/)
await check(
() => browser.elementById('slot').text(),
/Intercepted on Bar Page/
)
})
}
)