Provide non-dynamic segments to catch-all parallel routes (#65233)

Fixes an issue with https://github.com/vercel/next.js/pull/65063 where
the catch-all param only contains dynamic segments and is missing
non-dynamic route segments.

This makes building breadcrumbs extremely hard as we're missing
information to properly render the correct breadcrumb components.

This fix makes parallel route catch-all params behave like the standard
catch-all params in non-parallel routes

---------

Co-authored-by: Zack Tanner <1939140+ztanner@users.noreply.github.com>
This commit is contained in:
Andrew Gadzik 2024-05-01 18:32:45 -04:00 committed by GitHub
parent f2e7c8eeec
commit bb7f5a317f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 62 additions and 17 deletions

View file

@ -109,6 +109,7 @@ import {
} from '../client-component-renderer-logger'
import { createServerModuleMap } from './action-utils'
import { isNodeNextRequest } from '../base-http/helpers'
import { parseParameter } from '../../shared/lib/router/utils/route-regex'
export type GetDynamicParamFromSegment = (
// [slug] / [[slug]] / [...slug]
@ -207,6 +208,7 @@ export type CreateSegmentPath = (child: FlightSegmentPath) => FlightSegmentPath
*/
function makeGetDynamicParamFromSegment(
params: { [key: string]: any },
pagePath: string,
flightRouterState: FlightRouterState | undefined
): GetDynamicParamFromSegment {
return function getDynamicParamFromSegment(
@ -234,26 +236,46 @@ function makeGetDynamicParamFromSegment(
}
if (!value) {
// Handle case where optional catchall does not have a value, e.g. `/dashboard/[[...slug]]` when requesting `/dashboard`
if (
segmentParam.type === 'optional-catchall' ||
segmentParam.type === 'catchall'
) {
// If we weren't able to match the segment to a URL param, and we have a catch-all route,
// provide all of the known params (in array format) to the route
// It should be safe to assume the order of these params is consistent with the order of the segments.
// However, if not, we could re-parse the `pagePath` with `getRouteRegex` and iterate over the positional order.
value = Object.values(params).map((i) => encodeURIComponent(i))
const hasValues = value.length > 0
const type = dynamicParamTypes[segmentParam.type]
const isCatchall = segmentParam.type === 'catchall'
const isOptionalCatchall = segmentParam.type === 'optional-catchall'
if (isCatchall || isOptionalCatchall) {
const dynamicParamType = dynamicParamTypes[segmentParam.type]
// handle the case where an optional catchall does not have a value,
// e.g. `/dashboard/[[...slug]]` when requesting `/dashboard`
if (isOptionalCatchall) {
return {
param: key,
value: null,
type: dynamicParamType,
treeSegment: [key, '', dynamicParamType],
}
}
// handle the case where a catchall or optional catchall does not have a value,
// e.g. `/foo/bar/hello` and `@slot/[...catchall]` or `@slot/[[...catchall]]` is matched
value = pagePath
.split('/')
// remove the first empty string
.slice(1)
// replace any dynamic params with the actual values
.map((pathSegment) => {
const param = parseParameter(pathSegment)
// if the segment matches a param, return the param value
// otherwise, it's a static segment, so just return that
return params[param.key] ?? param.key
})
return {
param: key,
value: hasValues ? value : null,
type: type,
value,
type: dynamicParamType,
// This value always has to be a string.
treeSegment: [key, hasValues ? value.join('/') : '', type],
treeSegment: [key, value.join('/'), dynamicParamType],
}
}
return findDynamicParamFromRouterState(flightRouterState, segment)
}
@ -795,6 +817,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

View file

@ -25,7 +25,7 @@ export interface RouteRegex {
* - `[foo]` -> `{ key: 'foo', repeat: false, optional: true }`
* - `bar` -> `{ key: 'bar', repeat: false, optional: false }`
*/
function parseParameter(param: string) {
export function parseParameter(param: string) {
const optional = param.startsWith('[') && param.endsWith(']')
if (optional) {
param = param.slice(1, -1)

View file

@ -1,4 +1,4 @@
export default function Page({ params: { catchAll } }) {
export default function Page({ params: { catchAll = [] } }) {
return (
<div id="slot">
<h1>Parallel Route!</h1>

View file

@ -0,0 +1,7 @@
export default function StaticPage() {
return (
<div>
<h2>/foo/[lang]/bar Page!</h2>
</div>
)
}

View file

@ -44,4 +44,19 @@ describe('parallel-routes-breadcrumbs', () => {
expect(await slot.text()).toContain('Album: album2')
expect(await slot.text()).toContain('Track: track3')
})
it('should render the breadcrumbs correctly with the non-dynamic route segments', async () => {
const browser = await next.browser('/foo/en/bar')
const slot = await browser.waitForElementByCss('#slot')
expect(await browser.elementByCss('h1').text()).toBe('Parallel Route!')
expect(await browser.elementByCss('h2').text()).toBe(
'/foo/[lang]/bar Page!'
)
// verify slot is rendering the params
expect(await slot.text()).toContain('Artist: foo')
expect(await slot.text()).toContain('Album: en')
expect(await slot.text()).toContain('Track: bar')
})
})