fix dynamic param extraction for interception routes (#67400)

### What
When using `generateStaticParams` with interception routes, the
interception would never occur, and instead an MPA navigation would take
place to the targeted link.

### Why
For interception rewrites, we use a `__NEXT_EMPTY_PARAM__` marker (in
place of the actual param slot, eg `:locale`) for any params that are
discovered prior to the interception marker. This is because during
route resolution, the `params` for the interception route might not
contain the same `params` for the page that triggered the interception.
The dynamic params are then extracted from `FlightRouterState` at render
time. However, when `generateStaticParams` is present, the
`FlightRouterState` header is stripped from the request, so it isn't
able to extract the dynamic params and so the router thinks the new tree
is a new root layout, hence the MPA navigation.

### How
This removes the `__NEXT_EMPTY_PARAM__` hack and several spots where we
were forcing interception routes to be dynamic as a workaround to the
above bug. Now when resolving the route, if the request was to an
interception route, we extract the dynamic params from the request
before constructing the final rewritten URL. This will ensure that the
params from the "current" route are available in addition to the params
from the interception route without needing to defer until render.

Fixes #65192
Fixes #52880
This commit is contained in:
Zack Tanner 2024-07-03 11:21:07 -07:00 committed by GitHub
parent 13e501fc23
commit 1337c7a3e5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
18 changed files with 225 additions and 198 deletions

View file

@ -166,7 +166,6 @@ import {
import { getStartServerInfo, logStartInfo } from '../server/lib/app-info-log'
import type { NextEnabledDirectories } from '../server/base-server'
import { hasCustomExportOutput } from '../export/utils'
import { isInterceptionRouteAppPath } from '../server/lib/interception-routes'
import {
getTurbopackJsConfig,
handleEntrypoints,
@ -2106,8 +2105,6 @@ export default async function build(
}
const appConfig = workerResult.appConfig || {}
const isInterceptionRoute =
isInterceptionRouteAppPath(page)
if (appConfig.revalidate !== 0) {
const isDynamic = isDynamicRoute(page)
const hasGenerateStaticParams =
@ -2124,27 +2121,22 @@ export default async function build(
}
// Mark the app as static if:
// - It's not an interception route (these currently depend on request headers and cannot be computed at build)
// - It has no dynamic param
// - It doesn't have generateStaticParams but `dynamic` is set to
// `error` or `force-static`
if (!isInterceptionRoute) {
if (!isDynamic) {
appStaticPaths.set(originalAppPath, [page])
appStaticPathsEncoded.set(originalAppPath, [
page,
])
isStatic = true
} else if (
!hasGenerateStaticParams &&
(appConfig.dynamic === 'error' ||
appConfig.dynamic === 'force-static')
) {
appStaticPaths.set(originalAppPath, [])
appStaticPathsEncoded.set(originalAppPath, [])
isStatic = true
isRoutePPREnabled = false
}
if (!isDynamic) {
appStaticPaths.set(originalAppPath, [page])
appStaticPathsEncoded.set(originalAppPath, [page])
isStatic = true
} else if (
!hasGenerateStaticParams &&
(appConfig.dynamic === 'error' ||
appConfig.dynamic === 'force-static')
) {
appStaticPaths.set(originalAppPath, [])
appStaticPathsEncoded.set(originalAppPath, [])
isStatic = true
isRoutePPREnabled = false
}
}

View file

@ -61,8 +61,7 @@ import { unresolvedThenable } from './unresolved-thenable'
import { NEXT_RSC_UNION_QUERY } from './app-router-headers'
import { removeBasePath } from '../remove-base-path'
import { hasBasePath } from '../has-base-path'
import { PAGE_SEGMENT_KEY } from '../../shared/lib/segment'
import type { Params } from '../../shared/lib/router/utils/route-matcher'
import { getSelectedParams } from './router-reducer/compute-changed-path'
import type { FlightRouterState } from '../../server/app-render/types'
const isServer = typeof window === 'undefined'
@ -98,36 +97,6 @@ export function urlToUrlWithoutFlightMarker(url: string): URL {
return urlWithoutFlightParameters
}
// this function performs a depth-first search of the tree to find the selected
// params
function getSelectedParams(
currentTree: FlightRouterState,
params: Params = {}
): Params {
const parallelRoutes = currentTree[1]
for (const parallelRoute of Object.values(parallelRoutes)) {
const segment = parallelRoute[0]
const isDynamicParameter = Array.isArray(segment)
const segmentValue = isDynamicParameter ? segment[1] : segment
if (!segmentValue || segmentValue.startsWith(PAGE_SEGMENT_KEY)) continue
// Ensure catchAll and optional catchall are turned into an array
const isCatchAll =
isDynamicParameter && (segment[2] === 'c' || segment[2] === 'oc')
if (isCatchAll) {
params[segment[0]] = segment[1].split('/')
} else if (isDynamicParameter) {
params[segment[0]] = segment[1]
}
params = getSelectedParams(parallelRoute, params)
}
return params
}
type AppRouterProps = Omit<
Omit<InitialRouterStateParameters, 'isServer' | 'location'>,
'initialParallelRoutes'

View file

@ -3,6 +3,7 @@ import type {
Segment,
} from '../../../server/app-render/types'
import { INTERCEPTION_ROUTE_MARKERS } from '../../../server/lib/interception-routes'
import type { Params } from '../../../shared/lib/router/utils/route-matcher'
import {
isGroupSegment,
DEFAULT_SEGMENT_KEY,
@ -130,3 +131,34 @@ export function computeChangedPath(
// lightweight normalization to remove route groups
return normalizeSegments(changedPath.split('/'))
}
/**
* Recursively extracts dynamic parameters from FlightRouterState.
*/
export function getSelectedParams(
currentTree: FlightRouterState,
params: Params = {}
): Params {
const parallelRoutes = currentTree[1]
for (const parallelRoute of Object.values(parallelRoutes)) {
const segment = parallelRoute[0]
const isDynamicParameter = Array.isArray(segment)
const segmentValue = isDynamicParameter ? segment[1] : segment
if (!segmentValue || segmentValue.startsWith(PAGE_SEGMENT_KEY)) continue
// Ensure catchAll and optional catchall are turned into an array
const isCatchAll =
isDynamicParameter && (segment[2] === 'c' || segment[2] === 'oc')
if (isCatchAll) {
params[segment[0]] = segment[1].split('/')
} else if (isDynamicParameter) {
params[segment[0]] = segment[1]
}
params = getSelectedParams(parallelRoute, params)
}
return params
}

View file

@ -1,7 +1,6 @@
import { pathToRegexp } from 'next/dist/compiled/path-to-regexp'
import { NEXT_URL } from '../client/components/app-router-headers'
import {
INTERCEPTION_ROUTE_MARKERS,
extractInterceptionRouteInformation,
isInterceptionRouteAppPath,
} from '../server/lib/interception-routes'
@ -21,31 +20,6 @@ function toPathToRegexpPath(path: string): string {
})
}
// for interception routes we don't have access to the dynamic segments from the
// referrer route so we mark them as noop for the app renderer so that it
// can retrieve them from the router state later on. This also allows us to
// compile the route properly with path-to-regexp, otherwise it will throw
function voidParamsBeforeInterceptionMarker(path: string): string {
let newPath = []
let foundInterceptionMarker = false
for (const segment of path.split('/')) {
if (
INTERCEPTION_ROUTE_MARKERS.find((marker) => segment.startsWith(marker))
) {
foundInterceptionMarker = true
}
if (segment.startsWith(':') && !foundInterceptionMarker) {
newPath.push('__NEXT_EMPTY_PARAM__')
} else {
newPath.push(segment)
}
}
return newPath.join('/')
}
export function generateInterceptionRoutesRewrites(
appPaths: string[],
basePath = ''
@ -62,9 +36,7 @@ export function generateInterceptionRoutesRewrites(
}/(.*)?`
const normalizedInterceptedRoute = toPathToRegexpPath(interceptedRoute)
const normalizedAppPath = voidParamsBeforeInterceptionMarker(
toPathToRegexpPath(appPath)
)
const normalizedAppPath = toPathToRegexpPath(appPath)
// pathToRegexp returns a regex that matches the path, but we need to
// convert it to a string that can be used in a header value

View file

@ -34,7 +34,6 @@ import {
continueDynamicHTMLResume,
continueDynamicDataResume,
} from '../stream-utils/node-web-streams-helper'
import { canSegmentBeOverridden } from '../../client/components/match-segments'
import { stripInternalQueries } from '../internal-utils'
import {
NEXT_ROUTER_PREFETCH_HEADER,
@ -103,7 +102,6 @@ import {
StaticGenBailoutError,
isStaticGenBailoutError,
} from '../../client/components/static-generation-bailout'
import { isInterceptionRouteAppPath } from '../lib/interception-routes'
import { getStackWithoutErrorMessage } from '../../lib/format-server-error'
import {
usedDynamicAPIs,
@ -162,54 +160,6 @@ function createNotFoundLoaderTree(loaderTree: LoaderTree): LoaderTree {
return ['', {}, loaderTree[2]]
}
/* This method is important for intercepted routes to function:
* when a route is intercepted, e.g. /blog/[slug], it will be rendered
* with the layout of the previous page, e.g. /profile/[id]. The problem is
* that the loader tree needs to know the dynamic param in order to render (id and slug in the example).
* Normally they are read from the path but since we are intercepting the route, the path would not contain id,
* so we need to read it from the router state.
*/
function findDynamicParamFromRouterState(
flightRouterState: FlightRouterState | undefined,
segment: string
): {
param: string
value: string | string[] | null
treeSegment: Segment
type: DynamicParamTypesShort
} | null {
if (!flightRouterState) {
return null
}
const treeSegment = flightRouterState[0]
if (canSegmentBeOverridden(segment, treeSegment)) {
if (!Array.isArray(treeSegment) || Array.isArray(segment)) {
return null
}
return {
param: treeSegment[0],
value: treeSegment[1],
treeSegment: treeSegment,
type: treeSegment[2],
}
}
for (const parallelRouterState of Object.values(flightRouterState[1])) {
const maybeDynamicParam = findDynamicParamFromRouterState(
parallelRouterState,
segment
)
if (maybeDynamicParam) {
return maybeDynamicParam
}
}
return null
}
export type CreateSegmentPath = (child: FlightSegmentPath) => FlightSegmentPath
/**
@ -217,8 +167,7 @@ export type CreateSegmentPath = (child: FlightSegmentPath) => FlightSegmentPath
*/
function makeGetDynamicParamFromSegment(
params: { [key: string]: any },
pagePath: string,
flightRouterState: FlightRouterState | undefined
pagePath: string
): GetDynamicParamFromSegment {
return function getDynamicParamFromSegment(
// [slug] / [[slug]] / [...slug]
@ -233,11 +182,6 @@ function makeGetDynamicParamFromSegment(
let value = params[key]
// this is a special marker that will be present for interception routes
if (value === '__NEXT_EMPTY_PARAM__') {
value = undefined
}
if (Array.isArray(value)) {
value = value.map((i) => encodeURIComponent(i))
} else if (typeof value === 'string') {
@ -283,8 +227,6 @@ function makeGetDynamicParamFromSegment(
treeSegment: [key, value.join('/'), dynamicParamType],
}
}
return findDynamicParamFromRouterState(flightRouterState, segment)
}
const type = getShortDynamicParamType(segmentParam.type)
@ -820,16 +762,10 @@ async function renderToHTMLOrFlightImpl(
* Router state provided from the client-side router. Used to handle rendering
* from the common layout down. This value will be undefined if the request
* is not a client-side navigation request or if the request is a prefetch
* request (except when it's a prefetch request for an interception route
* which is always dynamic).
* request.
*/
const shouldProvideFlightRouterState =
isRSCRequest &&
(!isPrefetchRSCRequest ||
!isRoutePPREnabled ||
// Interception routes currently depend on the flight router state to
// extract dynamic params.
isInterceptionRouteAppPath(pagePath))
isRSCRequest && (!isPrefetchRSCRequest || !isRoutePPREnabled)
const parsedFlightRouterState = parseAndValidateFlightRouterState(
req.headers[NEXT_ROUTER_STATE_TREE.toLowerCase()]
@ -854,10 +790,7 @@ async function renderToHTMLOrFlightImpl(
const getDynamicParamFromSegment = makeGetDynamicParamFromSegment(
params,
pagePath,
// `FlightRouterState` is unconditionally provided here because this method uses it
// to extract dynamic params as a fallback if they're not present in the path.
parsedFlightRouterState
pagePath
)
// Get the nonce from the incoming request if it has one.

View file

@ -2,6 +2,15 @@ import type { FlightRouterState } from './types'
import { flightRouterStateSchema } from './types'
import { assert } from 'next/dist/compiled/superstruct'
export function parseAndValidateFlightRouterState(
stateHeader: string | string[]
): FlightRouterState
export function parseAndValidateFlightRouterState(
stateHeader: undefined
): undefined
export function parseAndValidateFlightRouterState(
stateHeader: string | string[] | undefined
): FlightRouterState | undefined
export function parseAndValidateFlightRouterState(
stateHeader: string | string[] | undefined
): FlightRouterState | undefined {

View file

@ -4,7 +4,7 @@ import type { NextConfigComplete } from '../../config-shared'
import type { RenderServer, initialize } from '../router-server'
import type { PatchMatcher } from '../../../shared/lib/router/utils/path-match'
import type { Redirect } from '../../../types'
import type { Header } from '../../../lib/load-custom-routes'
import type { Header, Rewrite } from '../../../lib/load-custom-routes'
import type { UnwrapPromise } from '../../../lib/coalesced-function'
import type { NextUrlWithParsedQuery } from '../../request-meta'
@ -37,6 +37,10 @@ import {
prepareDestination,
} from '../../../shared/lib/router/utils/prepare-destination'
import type { TLSSocket } from 'tls'
import { NEXT_ROUTER_STATE_TREE } from '../../../client/components/app-router-headers'
import { getSelectedParams } from '../../../client/components/router-reducer/compute-changed-path'
import { isInterceptionRouteRewrite } from '../../../lib/generate-interception-routes-rewrites'
import { parseAndValidateFlightRouterState } from '../../app-render/parse-and-validate-flight-router-state'
const debug = setupDebug('next:router-server:resolve-routes')
@ -689,10 +693,34 @@ export function getResolveRoutes(
// handle rewrite
if (route.destination) {
let rewriteParams = params
try {
// An interception rewrite might reference a dynamic param for a route the user
// is currently on, which wouldn't be extractable from the matched route params.
// This attempts to extract the dynamic params from the provided router state.
if (isInterceptionRouteRewrite(route as Rewrite)) {
const stateHeader =
req.headers[NEXT_ROUTER_STATE_TREE.toLowerCase()]
if (stateHeader) {
rewriteParams = {
...getSelectedParams(
parseAndValidateFlightRouterState(stateHeader)
),
...params,
}
}
}
} catch (err) {
// this is a no-op -- we couldn't extract dynamic params from the provided router state,
// so we'll just use the params from the route matcher
}
const { parsedDestination } = prepareDestination({
appendParamsToQuery: true,
destination: route.destination,
params: params,
params: rewriteParams,
query: parsedUrl.query,
})

View file

@ -1,7 +1,6 @@
import { nextTestSetup, FileRef } from 'e2e-utils'
import { retry } from 'next-test-utils'
import { join } from 'path'
import { Response } from 'playwright'
describe('interception-route-prefetch-cache', () => {
function runTests({ next }: ReturnType<typeof nextTestSetup>) {
@ -53,49 +52,11 @@ describe('interception-route-prefetch-cache', () => {
}
describe('runtime = nodejs', () => {
const testSetup = nextTestSetup({
files: __dirname,
})
runTests(testSetup)
const { next, isNextStart } = testSetup
// this is a node runtime specific test as edge doesn't support static rendering
if (isNextStart) {
it('should not be a cache HIT when prefetching an interception route', async () => {
const responses: { cacheStatus: string; pathname: string }[] = []
const browser = await next.browser('/baz', {
beforePageLoad(page) {
page.on('response', (response: Response) => {
const url = new URL(response.url())
const request = response.request()
const responseHeaders = response.headers()
const requestHeaders = request.headers()
if (requestHeaders['next-router-prefetch']) {
responses.push({
cacheStatus: responseHeaders['x-nextjs-cache'],
pathname: url.pathname,
})
}
})
},
})
expect(await browser.elementByCss('body').text()).toContain(
'Open Interception Modal'
)
const interceptionPrefetchResponse = responses.find(
(response) => response.pathname === '/baz/modal'
)
const homePrefetchResponse = responses.find(
(response) => response.pathname === '/'
)
expect(homePrefetchResponse.cacheStatus).toBe('HIT') // sanity check to ensure we're seeing cache statuses
expect(interceptionPrefetchResponse.cacheStatus).toBeUndefined()
runTests(
nextTestSetup({
files: __dirname,
})
}
)
})
describe('runtime = edge', () => {

View file

@ -0,0 +1,13 @@
export default function ModalPage({
params: { id },
}: {
params: { id: string }
}) {
return (
<dialog id="intercepted-slot" open>
<h2>Modal for Interception Page</h2>
<p>Using route interception</p>
<p>Param: {id}</p>
</dialog>
)
}

View file

@ -0,0 +1,5 @@
const Default = () => {
return null
}
export default Default

View file

@ -0,0 +1,12 @@
export default function ModalPage({
params: { id },
}: {
params: { id: string }
}) {
return (
<dialog id="non-intercepted-slot" open>
<h1>Modal for No Interception Page</h1>
<p>Param: {id}</p>
</dialog>
)
}

View file

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

View file

@ -0,0 +1,11 @@
import Link from 'next/link'
export default function Page({ params: { id } }: { params: { id: string } }) {
return (
<div id="intercepted-page">
<h1>Interception Page</h1>
<p>Param: {id}</p>
<Link href="/">Back</Link>
</div>
)
}

View file

@ -0,0 +1,23 @@
const locales = ['en', 'de', 'fr', 'it', 'nl', 'es', 'pt', 'pl']
//Needed for static rendering
export function generateStaticParams() {
return locales.map((locale) => ({ locale }))
}
export default function RootLayout({
children,
modal,
}: {
children: React.ReactNode
modal: React.ReactNode
}) {
return (
<html lang="en">
<body>
{children}
{modal}
</body>
</html>
)
}

View file

@ -0,0 +1,12 @@
import Link from 'next/link'
export default function Page({ params: { id } }: { params: { id: string } }) {
return (
<div id="non-intercepted-page">
<h1>No Interception Page</h1>
<p>No route interception</p>
<p>Param: {id}</p>
<Link href="/">Back</Link>
</div>
)
}

View file

@ -0,0 +1,17 @@
import Link from 'next/link'
export default function Home({
params: { locale },
}: {
params: { locale: string }
}) {
return (
<div id="home-page">
<h1>Home Page</h1>
<br />
<Link href={`/${locale}/interception/123`}>Interception link</Link>
<br />
<Link href={`/${locale}/no-interception/123`}>No interception link</Link>
</div>
)
}

View file

@ -0,0 +1,6 @@
/**
* @type {import('next').NextConfig}
*/
const nextConfig = {}
module.exports = nextConfig

View file

@ -0,0 +1,29 @@
import { nextTestSetup } from 'e2e-utils'
describe('parallel-routes-generate-static-params', () => {
const { next } = nextTestSetup({
files: __dirname,
})
it('should render the intercepted/non-intercepted modal', async () => {
const browser = await next.browser('/en')
expect(await browser.elementByCss('h1').text()).toBe('Home Page')
await browser.elementByCss("[href='/en/interception/123']").click()
await browser.waitForElementByCss('#intercepted-slot')
expect(await browser.elementByCss('h1').text()).toBe('Home Page')
expect(await browser.elementByCss('h2').text()).toBe(
'Modal for Interception Page'
)
await browser.back()
await browser.waitForElementByCss('#home-page')
await browser.elementByCss("[href='/en/no-interception/123']").click()
await browser.waitForElementByCss('#non-intercepted-page')
expect(await browser.elementByCss('h1').text()).toBe('No Interception Page')
})
})