Skip to content

Commit

Permalink
fix revalidation/refresh behavior with parallel routes (#63608)
Browse files Browse the repository at this point in the history
### What
When a parallel segment in the current router tree is no longer "active"
during a soft navigation (ie, no longer matches a page component on the
particular route), it remains on-screen until the page is refreshed, at
which point it would switch to rendering the `default.tsx` component.
However, when revalidating the router cache via `router.refresh`, or
when a server action finishes & refreshes the router cache, this would
trigger the "hard refresh" behavior. This would have the unintended
consequence of a 404 being triggered (which is the default behavior of
`default.tsx`) or inactive segments disappearing unexpectedly.

### Why
When the router cache is refreshed, it currently fetches new data for
the page by fetching from the current URL the user is on. This means
that the server will never respond with the data it needs if the segment
wasn't "activated" via the URL we're fetching from, as it came from
someplace else. Instead, the server will give us data for the
`default.tsx` component, which we don't want to render when doing a soft
refresh.

### How
This updates the `FlightRouterState` to encode information about the URL
that caused the segment to become active. That way, when some sort of
revalidation event takes place, we can both refresh the data for the
current URL (existing handling), and recursively refetch segment data
for anything that was still present in the tree but requires fetching
from a different URL. We patch this new data into the tree before
committing the final `CacheNode` to the router.

**Note**: I re-used the existing `refresh` and `url` arguments in
`FlightRouterState` to avoid introducing more options to this data
structure that is already a bit tricky to work with. Initially I was
going to re-use `"refetch"` as-is, which seemed to work ok, but I'm
worried about potential implications of this considering they have
different semantics. In an abundance of caution, I added a new marker
type ("`refresh`", alternative suggestions welcome).

This has some trade-offs: namely, if there are a lot of different
segments that are in this stale state that require data from different
URLs, the refresh is going to be blocked while we fetch all of these
segments. Having to do a separate round-trip for each of these segments
could be expensive. In an ideal world, we'd be able to enumerate the
segments we'd want to refetch and where they came from, so it could be
handled in a single round-trip. There are some ideas on how to improve
per-segment fetching which are out of scope of this PR. However, due to
the implicit contract that `middleware.ts` creates with URLs, we still
need to identify these resources by URLs.

Fixes #60815
Fixes #60950
Fixes #51711
Fixes #51714
Fixes #58715
Fixes #60948
Fixes #62213
Fixes #61341

Closes NEXT-1845
Closes NEXT-2030
  • Loading branch information
ztanner committed Mar 28, 2024
1 parent 62f8753 commit 42e6cc6
Show file tree
Hide file tree
Showing 24 changed files with 557 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {
} from '../../../server/app-render/types'
import { DEFAULT_SEGMENT_KEY } from '../../../shared/lib/segment'
import { matchSegment } from '../match-segments'
import { addRefreshMarkerToActiveParallelSegments } from './refetch-inactive-parallel-segments'

/**
* Deep merge of the two router states. Parallel route keys are preserved if the patch doesn't have them.
Expand Down Expand Up @@ -139,5 +140,7 @@ export function applyRouterStatePatchToTree(
tree[4] = true
}

addRefreshMarkerToActiveParallelSegments(tree, pathname)

return tree
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { fillLazyItemsTillLeafWithHead } from './fill-lazy-items-till-leaf-with-
import { extractPathFromFlightRouterState } from './compute-changed-path'
import { createPrefetchCacheEntryForInitialLoad } from './prefetch-cache-utils'
import { PrefetchKind, type PrefetchCacheEntry } from './router-reducer-types'
import { addRefreshMarkerToActiveParallelSegments } from './refetch-inactive-parallel-segments'

export interface InitialRouterStateParameters {
buildId: string
Expand Down Expand Up @@ -48,6 +49,8 @@ export function createInitialRouterState({
loading: initialSeedData[3],
}

addRefreshMarkerToActiveParallelSegments(initialTree, initialCanonicalUrl)

const prefetchCache = new Map<string, PrefetchCacheEntry>()

// When the cache hasn't been seeded yet we fill the cache with the head.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { fillLazyItemsTillLeafWithHead } from '../fill-lazy-items-till-leaf-with
import { createEmptyCacheNode } from '../../app-router'
import { handleSegmentMismatch } from '../handle-segment-mismatch'
import { hasInterceptionRouteInCurrentTree } from './has-interception-route-in-current-tree'
import { refreshInactiveParallelSegments } from '../refetch-inactive-parallel-segments'

export function refreshReducer(
state: ReadonlyReducerState,
Expand Down Expand Up @@ -44,7 +45,7 @@ export function refreshReducer(
)

return cache.lazyData.then(
([flightData, canonicalUrlOverride]) => {
async ([flightData, canonicalUrlOverride]) => {
// Handle case when navigating to page in `pages` from `app`
if (typeof flightData === 'string') {
return handleExternalUrl(
Expand Down Expand Up @@ -113,10 +114,17 @@ export function refreshReducer(
cacheNodeSeedData,
head
)
mutable.cache = cache
mutable.prefetchCache = new Map()
}

await refreshInactiveParallelSegments({
state,
updatedTree: newTree,
updatedCache: cache,
includeNextUrl,
})

mutable.cache = cache
mutable.patchedTree = newTree
mutable.canonicalUrl = href

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { fillLazyItemsTillLeafWithHead } from '../fill-lazy-items-till-leaf-with
import { createEmptyCacheNode } from '../../app-router'
import { hasInterceptionRouteInCurrentTree } from './has-interception-route-in-current-tree'
import { handleSegmentMismatch } from '../handle-segment-mismatch'
import { refreshInactiveParallelSegments } from '../refetch-inactive-parallel-segments'

type FetchServerActionResult = {
redirectLocation: URL | undefined
Expand All @@ -53,17 +54,11 @@ type FetchServerActionResult = {

async function fetchServerAction(
state: ReadonlyReducerState,
nextUrl: ReadonlyReducerState['nextUrl'],
{ actionId, actionArgs }: ServerActionAction
): Promise<FetchServerActionResult> {
const body = await encodeReply(actionArgs)

// only pass along the `nextUrl` param (used for interception routes) if the current route was intercepted.
// If the route has been intercepted, the action should be as well.
// Otherwise the server action might be intercepted with the wrong action id
// (ie, one that corresponds with the intercepted route)
const includeNextUrl =
state.nextUrl && hasInterceptionRouteInCurrentTree(state.tree)

const res = await fetch('', {
method: 'POST',
headers: {
Expand All @@ -75,9 +70,9 @@ async function fetchServerAction(
'x-deployment-id': process.env.NEXT_DEPLOYMENT_ID,
}
: {}),
...(includeNextUrl
...(nextUrl
? {
[NEXT_URL]: state.nextUrl,
[NEXT_URL]: nextUrl,
}
: {}),
},
Expand Down Expand Up @@ -162,10 +157,24 @@ export function serverActionReducer(
let currentTree = state.tree

mutable.preserveCustomHistoryState = false
mutable.inFlightServerAction = fetchServerAction(state, action)

// only pass along the `nextUrl` param (used for interception routes) if the current route was intercepted.
// If the route has been intercepted, the action should be as well.
// Otherwise the server action might be intercepted with the wrong action id
// (ie, one that corresponds with the intercepted route)
const nextUrl =
state.nextUrl && hasInterceptionRouteInCurrentTree(state.tree)
? state.nextUrl
: null

mutable.inFlightServerAction = fetchServerAction(state, nextUrl, action)

return mutable.inFlightServerAction.then(
({ actionResult, actionFlightData: flightData, redirectLocation }) => {
async ({
actionResult,
actionFlightData: flightData,
redirectLocation,
}) => {
// Make sure the redirection is a push instead of a replace.
// Issue: https://github.com/vercel/next.js/issues/53911
if (redirectLocation) {
Expand Down Expand Up @@ -249,6 +258,14 @@ export function serverActionReducer(
cacheNodeSeedData,
head
)

await refreshInactiveParallelSegments({
state,
updatedTree: newTree,
updatedCache: cache,
includeNextUrl: Boolean(nextUrl),
})

mutable.cache = cache
mutable.prefetchCache = new Map()
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
import type { FlightRouterState } from '../../../server/app-render/types'
import type { CacheNode } from '../../../shared/lib/app-router-context.shared-runtime'
import type { AppRouterState } from './router-reducer-types'
import { applyFlightData } from './apply-flight-data'
import { fetchServerResponse } from './fetch-server-response'
import { PAGE_SEGMENT_KEY } from '../../../shared/lib/segment'

interface RefreshInactiveParallelSegments {
state: AppRouterState
updatedTree: FlightRouterState
updatedCache: CacheNode
includeNextUrl: boolean
}

/**
* Refreshes inactive segments that are still in the current FlightRouterState.
* A segment is considered "inactive" when the server response indicates it didn't match to a page component.
* This happens during a soft-navigation, where the server will want to patch in the segment
* with the "default" component, but we explicitly ignore the server in this case
* and keep the existing state for that segment. New data for inactive segments are inherently
* not part of the server response when we patch the tree, because they were associated with a response
* from an earlier navigation/request. For each segment, once it becomes "active", we encode the URL that provided
* the data for it. This function traverses parallel routes looking for these markers so that it can re-fetch
* and patch the new data into the tree.
*/
export async function refreshInactiveParallelSegments(
options: RefreshInactiveParallelSegments
) {
const fetchedSegments = new Set<string>()
await refreshInactiveParallelSegmentsImpl({ ...options, fetchedSegments })
}

async function refreshInactiveParallelSegmentsImpl({
state,
updatedTree,
updatedCache,
includeNextUrl,
fetchedSegments,
}: RefreshInactiveParallelSegments & { fetchedSegments: Set<string> }) {
const [, parallelRoutes, refetchPathname, refetchMarker] = updatedTree
const fetchPromises = []

if (
refetchPathname &&
refetchPathname !== location.pathname &&
refetchMarker === 'refresh' &&
// it's possible for the tree to contain multiple segments that contain data at the same URL
// we keep track of them so we can dedupe the requests
!fetchedSegments.has(refetchPathname)
) {
fetchedSegments.add(refetchPathname) // Mark this URL as fetched

// Eagerly kick off the fetch for the refetch path & the parallel routes. This should be fine to do as they each operate
// independently on their own cache nodes, and `applyFlightData` will copy anything it doesn't care about from the existing cache.
const fetchPromise = fetchServerResponse(
// we capture the pathname of the refetch without search params, so that it can be refetched with
// the "latest" search params when it comes time to actually trigger the fetch (below)
new URL(refetchPathname + location.search, location.origin),
[updatedTree[0], updatedTree[1], updatedTree[2], 'refetch'],
includeNextUrl ? state.nextUrl : null,
state.buildId
).then((fetchResponse) => {
const flightData = fetchResponse[0]
if (typeof flightData !== 'string') {
for (const flightDataPath of flightData) {
// we only pass the new cache as this function is called after clearing the router cache
// and filling in the new page data from the server. Meaning the existing cache is actually the cache that's
// just been created & has been written to, but hasn't been "committed" yet.
applyFlightData(updatedCache, updatedCache, flightDataPath)
}
} else {
// When flightData is a string, it suggests that the server response should have triggered an MPA navigation
// I'm not 100% sure of this decision, but it seems unlikely that we'd want to introduce a redirect side effect
// when refreshing on-screen data, so handling this has been ommitted.
}
})

fetchPromises.push(fetchPromise)
}

for (const key in parallelRoutes) {
const parallelFetchPromise = refreshInactiveParallelSegmentsImpl({
state,
updatedTree: parallelRoutes[key],
updatedCache,
includeNextUrl,
fetchedSegments,
})

fetchPromises.push(parallelFetchPromise)
}

await Promise.all(fetchPromises)
}

/**
* Walks the current parallel segments to determine if they are "active".
* An active parallel route will have a `__PAGE__` segment in the FlightRouterState.
* As opposed to a `__DEFAULT__` segment, which means there was no match for that parallel route.
* We add a special marker here so that we know how to refresh its data when the router is revalidated.
*/
export function addRefreshMarkerToActiveParallelSegments(
tree: FlightRouterState,
pathname: string
) {
const [segment, parallelRoutes, , refetchMarker] = tree
if (segment === PAGE_SEGMENT_KEY && refetchMarker !== 'refresh') {
tree[2] = pathname
tree[3] = 'refresh'
}

for (const key in parallelRoutes) {
addRefreshMarkerToActiveParallelSegments(parallelRoutes[key], pathname)
}
}
10 changes: 8 additions & 2 deletions packages/next/src/server/app-render/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const flightRouterStateSchema: s.Describe<any> = s.tuple([
s.lazy(() => flightRouterStateSchema)
),
s.optional(s.nullable(s.string())),
s.optional(s.nullable(s.literal('refetch'))),
s.optional(s.nullable(s.union([s.literal('refetch'), s.literal('refresh')]))),
s.optional(s.boolean()),
])

Expand All @@ -49,7 +49,13 @@ export type FlightRouterState = [
segment: Segment,
parallelRoutes: { [parallelRouterKey: string]: FlightRouterState },
url?: string | null,
refresh?: 'refetch' | null,
/*
/* "refresh" and "refetch", despite being similarly named, have different semantics.
* - "refetch" is a server indicator which informs where rendering should start from.
* - "refresh" is a client router indicator that it should re-fetch the data from the server for the current segment.
* It uses the "url" property above to determine where to fetch from.
*/
refresh?: 'refetch' | 'refresh' | null,
isRootLayout?: boolean
]

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return null
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/parallel-routes-revalidation/app/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ export default function Root({
}) {
return (
<html>
<head>
<script src="https://cdn.tailwindcss.com?plugins=typography"></script>
</head>
<body>
{children}
{dialog}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Default() {
return null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
'use client'

import Link from 'next/link'
import { useRouter } from 'next/navigation'
import { revalidateAction } from '../../@modal/modal/action'

export default function Page() {
const router = useRouter()

const handleRevalidateSubmit = async () => {
const result = await revalidateAction()
if (result.success) {
close()
}
}

const close = () => {
router.back()
}

return (
<div className="w-1/3 fixed right-0 top-0 bottom-0 h-screen shadow-2xl bg-gray-50 p-10">
<h2 id="drawer">Drawer</h2>
<p id="drawer-now">{Date.now()}</p>

<button
type="button"
id="drawer-close-button"
onClick={() => close()}
className="bg-gray-100 border p-2 rounded"
>
close
</button>
<p className="mt-4">Drawer</p>
<div className="mt-4 flex flex-col gap-2">
<Link
href="/nested-revalidate/modal"
className="bg-sky-600 text-white p-2 rounded"
>
Open modal
</Link>
<form action={handleRevalidateSubmit}>
<button
type="submit"
className="bg-sky-600 text-white p-2 rounded"
id="drawer-submit-button"
>
Revalidate submit
</button>
</form>
</div>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Default() {
return null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use server'

import { revalidatePath } from 'next/cache'

export async function revalidateAction() {
console.log('revalidate action')
revalidatePath('/')
return {
success: true,
}
}

0 comments on commit 42e6cc6

Please sign in to comment.