Skip to content

Commit

Permalink
only prefix prefetch cache entries if they vary based on Next-URL (#…
Browse files Browse the repository at this point in the history
…61235)

### What
Prefetches to pages within a shared layout would frequently cache miss
despite having the data available. This causes the "instant navigation"
behavior (with the 30s/5min TTL) to not be effective on these pages.

### Why
In #59861, `nextUrl` was added as a prefetch cache key prefix to ensure
multiple interception routes that correspond to the same URL wouldn't
clash in the prefetch cache. However this causes a problem in the case
where you're navigating between sub-pages. To illustrate the issue,
consider the case where you load `/foo`. This will populate the prefetch
cache with an entry of `{foo: <PrefetchCacheNode}`. Navigating to
`/foo/bar`, with a link that prefetches back to `/foo`, will now result
in a new cache node: `{foo: <PrefetchCacheNode>, /foo/bar%/foo:
<PrefetchCacheNode>}` (where `Next-URL` is `/foo/bar`). Now we have a
cache entry for the full data, as well as a cache entry for a partial
prefetch up to the nearest loading boundary. Now when we navigate back
to `/foo`, the router will see that it's missing data, and need to
lazy-fetch the data triggering the loading boundary.

This was especially noticeable in the case where you have a route group
with it's own loading.js file because it creates a level of hierarchy in
the React tree, and suspending on the data fetch would result in the
group's loading boundary to be triggered. In the non-route group
scenario, there's still a bug here but it would stall on the data fetch
rather than triggering a boundary.

### How
In #61794 we conditionally send `Next-URL` as part of the `Vary` header
if we detect it could be intercepted. We use this information when
creating the prefetch entry to prefix it, in case it corresponds with an
intercepted route.

Closes NEXT-2193
  • Loading branch information
ztanner authored Feb 13, 2024
1 parent dc88609 commit b9861fd
Show file tree
Hide file tree
Showing 16 changed files with 345 additions and 152 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,12 @@ import { hexHash } from '../../../shared/lib/hash'
export type FetchServerResponseResult = [
flightData: FlightData,
canonicalUrlOverride: URL | undefined,
postponed?: boolean
postponed?: boolean,
intercepted?: boolean
]

function doMpaNavigation(url: string): FetchServerResponseResult {
return [urlToUrlWithoutFlightMarker(url).toString(), undefined]
return [urlToUrlWithoutFlightMarker(url).toString(), undefined, false, false]
}

/**
Expand Down Expand Up @@ -112,6 +113,7 @@ export async function fetchServerResponse(

const contentType = res.headers.get('content-type') || ''
const postponed = !!res.headers.get(NEXT_DID_POSTPONE_HEADER)
const interception = !!res.headers.get('vary')?.includes(NEXT_URL)
let isFlightResponse = contentType === RSC_CONTENT_TYPE_HEADER

if (process.env.NODE_ENV === 'production') {
Expand Down Expand Up @@ -145,7 +147,7 @@ export async function fetchServerResponse(
return doMpaNavigation(res.url)
}

return [flightData, canonicalUrl, postponed]
return [flightData, canonicalUrl, postponed, interception]
} catch (err) {
console.error(
`Failed to fetch RSC payload for ${url}. Falling back to browser navigation.`,
Expand All @@ -154,6 +156,6 @@ export async function fetchServerResponse(
// If fetch fails handle it like a mpa navigation
// TODO-APP: Add a test for the case where a CORS request fails, e.g. external url redirect coming from the response.
// See https://github.com/vercel/next.js/issues/43605#issuecomment-1451617521 for a reproduction.
return [url.toString(), undefined]
return [url.toString(), undefined, false, false]
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { createHrefFromUrl } from './create-href-from-url'
import { fetchServerResponse } from './fetch-server-response'
import {
PrefetchCacheEntryStatus,
type AppRouterState,
type PrefetchCacheEntry,
PrefetchKind,
type ReadonlyReducerState,
} from './router-reducer-types'
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'
import { prefetchQueue } from './reducers/prefetch-reducer'

/**
* Creates a cache key for the router prefetch cache
Expand All @@ -14,27 +15,177 @@ import { createHrefFromUrl } from './create-href-from-url'
* @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) {
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)
// nextUrl is used as a cache key delimiter since entries can vary based on the Next-URL header
if (nextUrl) {
return `${nextUrl}%${pathnameFromUrl}`
}

return pathnameFromUrl
}

/**
* Returns a prefetch cache entry if one exists. Otherwise creates a new one and enqueues a fetch request
* to retrieve the prefetch data from the server.
*/
export function getOrCreatePrefetchCacheEntry({
url,
nextUrl,
tree,
buildId,
prefetchCache,
kind,
}: Pick<
ReadonlyReducerState,
'nextUrl' | 'prefetchCache' | 'tree' | 'buildId'
> & {
url: URL
kind?: PrefetchKind
}): PrefetchCacheEntry {
let existingCacheEntry: PrefetchCacheEntry | undefined = undefined
// We first check if there's a more specific interception route prefetch entry
// This is because when we detect a prefetch that corresponds with an interception route, we prefix it with nextUrl (see `createPrefetchCacheKey`)
// to avoid conflicts with other pages that may have the same URL but render different things depending on the `Next-URL` header.
const interceptionCacheKey = createPrefetchCacheKey(url, nextUrl)
const interceptionData = prefetchCache.get(interceptionCacheKey)

if (interceptionData) {
existingCacheEntry = interceptionData
} else {
// If we dont find a more specific interception route prefetch entry, we check for a regular prefetch entry
const prefetchCacheKey = createPrefetchCacheKey(url)
const prefetchData = prefetchCache.get(prefetchCacheKey)
if (prefetchData) {
existingCacheEntry = prefetchData
}
}

if (existingCacheEntry) {
// when `kind` is provided, an explicit prefetch was requested.
// if the requested prefetch is "full" and the current cache entry wasn't, we want to re-prefetch with the new intent
if (
kind &&
existingCacheEntry.kind !== PrefetchKind.FULL &&
kind === PrefetchKind.FULL
) {
return createLazyPrefetchEntry({
tree,
url,
buildId,
nextUrl,
prefetchCache,
kind,
})
}

// Grab the latest status of the cache entry and update it
existingCacheEntry.status = getPrefetchEntryCacheStatus(existingCacheEntry)

// If the existing cache entry was marked as temporary, it means it was lazily created when attempting to get an entry,
// where we didn't have the prefetch intent. Now that we have the intent (in `kind`), we want to update the entry to the more accurate kind.
if (kind && existingCacheEntry.kind === PrefetchKind.TEMPORARY) {
existingCacheEntry.kind = kind
}

// We've determined that the existing entry we found is still valid, so we return it.
return existingCacheEntry
}

// If we didn't return an entry, create a new one.
return createLazyPrefetchEntry({
tree,
url,
buildId,
nextUrl,
prefetchCache,
kind:
kind ||
// in dev, there's never gonna be a prefetch entry so we want to prefetch here
(process.env.NODE_ENV === 'development'
? PrefetchKind.AUTO
: PrefetchKind.TEMPORARY),
})
}

function prefixExistingPrefetchCacheEntry({
url,
nextUrl,
prefetchCache,
}: Pick<ReadonlyReducerState, 'nextUrl' | 'prefetchCache'> & {
url: URL
}) {
const existingCacheKey = createPrefetchCacheKey(url)
const existingCacheEntry = prefetchCache.get(existingCacheKey)
if (!existingCacheEntry) {
// no-op -- there wasn't an entry to move
return
}

const newCacheKey = createPrefetchCacheKey(url, nextUrl)
prefetchCache.set(newCacheKey, existingCacheEntry)
prefetchCache.delete(existingCacheKey)
}

/**
* Creates a prefetch entry for data that has not been resolved. This will add the prefetch request to a promise queue.
*/
function createLazyPrefetchEntry({
url,
kind,
tree,
nextUrl,
buildId,
prefetchCache,
}: Pick<
ReadonlyReducerState,
'nextUrl' | 'tree' | 'buildId' | 'prefetchCache'
> & {
url: URL
kind: PrefetchKind
}): PrefetchCacheEntry {
const prefetchCacheKey = createPrefetchCacheKey(url)

// initiates the fetch request for the prefetch and attaches a listener
// to the promise to update the prefetch cache entry when the promise resolves (if necessary)
const data = prefetchQueue.enqueue(() =>
fetchServerResponse(url, tree, nextUrl, buildId, kind).then(
(prefetchResponse) => {
// TODO: `fetchServerResponse` should be more tighly coupled to these prefetch cache operations
// to avoid drift between this cache key prefixing logic
// (which is currently directly influenced by the server response)
const [, , , intercepted] = prefetchResponse
if (intercepted) {
prefixExistingPrefetchCacheEntry({ url, nextUrl, prefetchCache })
}

return prefetchResponse
}
)
)

const prefetchEntry = {
treeAtTimeOfPrefetch: tree,
data,
kind,
prefetchTime: Date.now(),
lastUsedTime: null,
key: prefetchCacheKey,
status: PrefetchCacheEntryStatus.fresh,
}

prefetchCache.set(prefetchCacheKey, prefetchEntry)

return prefetchEntry
}

export function prunePrefetchCache(
prefetchCache: AppRouterState['prefetchCache']
prefetchCache: ReadonlyReducerState['prefetchCache']
) {
for (const [href, prefetchCacheEntry] of prefetchCache) {
if (
Expand All @@ -49,7 +200,7 @@ export function prunePrefetchCache(
const FIVE_MINUTES = 5 * 60 * 1000
const THIRTY_SECONDS = 30 * 1000

export function getPrefetchEntryCacheStatus({
function getPrefetchEntryCacheStatus({
kind,
prefetchTime,
lastUsedTime,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,9 +256,11 @@ describe('navigateReducer', () => {
"prefetchCache": Map {
"/linking/about" => {
"data": Promise {},
"key": "/linking/about",
"kind": "temporary",
"lastUsedTime": 1690329600000,
"prefetchTime": 1690329600000,
"status": "fresh",
"treeAtTimeOfPrefetch": [
"",
{
Expand Down Expand Up @@ -450,9 +452,11 @@ describe('navigateReducer', () => {
"prefetchCache": Map {
"/linking/about" => {
"data": Promise {},
"key": "/linking/about",
"kind": "temporary",
"lastUsedTime": 1690329600000,
"prefetchTime": 1690329600000,
"status": "fresh",
"treeAtTimeOfPrefetch": [
"",
{
Expand Down Expand Up @@ -887,9 +891,11 @@ describe('navigateReducer', () => {
"prefetchCache": Map {
"/linking" => {
"data": Promise {},
"key": "/linking",
"kind": "temporary",
"lastUsedTime": 1690329600000,
"prefetchTime": 1690329600000,
"status": "fresh",
"treeAtTimeOfPrefetch": [
"",
{
Expand Down Expand Up @@ -1113,9 +1119,11 @@ describe('navigateReducer', () => {
"prefetchCache": Map {
"/linking/about" => {
"data": Promise {},
"key": "/linking/about",
"kind": "auto",
"lastUsedTime": 1690329600000,
"prefetchTime": 1690329600000,
"status": "fresh",
"treeAtTimeOfPrefetch": [
"",
{
Expand Down Expand Up @@ -1367,9 +1375,11 @@ describe('navigateReducer', () => {
"prefetchCache": Map {
"/parallel-tab-bar/demographics" => {
"data": Promise {},
"key": "/parallel-tab-bar/demographics",
"kind": "temporary",
"lastUsedTime": 1690329600000,
"prefetchTime": 1690329600000,
"status": "fresh",
"treeAtTimeOfPrefetch": [
"",
{
Expand Down Expand Up @@ -1710,9 +1720,11 @@ describe('navigateReducer', () => {
"prefetchCache": Map {
"/linking/about" => {
"data": Promise {},
"key": "/linking/about",
"kind": "temporary",
"lastUsedTime": 1690329600000,
"prefetchTime": 1690329600000,
"status": "fresh",
"treeAtTimeOfPrefetch": [
"",
{
Expand Down
Loading

0 comments on commit b9861fd

Please sign in to comment.