Skip to content

Commit

Permalink
Make catchall parameters consistent with existing router (#38456)
Browse files Browse the repository at this point in the history
Ensures catchall parameters are passed as an array to `params` instead of as a string.


## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm lint`
- [ ] The examples guidelines are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing.md#adding-examples)
  • Loading branch information
timneutkens authored Jul 8, 2022
1 parent a4668f2 commit 050d13e
Showing 1 changed file with 33 additions and 32 deletions.
65 changes: 33 additions & 32 deletions packages/next/server/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,11 @@ export async function renderToHTML(
const getDynamicParamFromSegment = (
// [id] or [slug]
segment: string
): { param: string; value: string } | null => {
): {
param: string
value: string | string[] | null
treeValue: string
} | null => {
// TODO: use correct matching for dynamic routes to get segment param
const segmentParam = getSegmentParam(segment)
if (!segmentParam) {
Expand All @@ -422,24 +426,20 @@ export async function renderToHTML(
if (segmentParam.type === 'optional-catchall') {
return {
param: key,
value: '',
value: null,
treeValue: '',
}
}
return null
}

// TODO: this should only read from `pathParams`. There's an inconsistency where `query` holds params currently which has to be fixed.
const value = pathParams[key] ?? query[key]

return {
param: key,
value:
// TODO: this should only read from `pathParams`. There's an inconsistency where `query` holds params currently which has to be fixed.
(Array.isArray(pathParams[key])
? // @ts-expect-error TODO: handle case where value is an array
pathParams[key].join('/')
: pathParams[key]) ??
(Array.isArray(query[key])
? // @ts-expect-error TODO: handle case where value is an array
query[key].join('/')
: query[key]),
value: value,
treeValue: Array.isArray(value) ? value.join('/') : value,
}
}

Expand All @@ -450,7 +450,7 @@ export async function renderToHTML(
const dynamicParam = getDynamicParamFromSegment(segment)

const segmentTree: FlightRouterState = [
dynamicParam ? [dynamicParam.param, dynamicParam.value] : segment,
dynamicParam ? [dynamicParam.param, dynamicParam.treeValue] : segment,
{},
]

Expand Down Expand Up @@ -506,16 +506,16 @@ export async function renderToHTML(
: undefined

const segmentParam = getDynamicParamFromSegment(segment)

const currentParams = segmentParam
? {
...parentParams,
[segmentParam.param]: segmentParam.value,
}
: parentParams

const currentParams =
// Handle null case where dynamic param is optional
segmentParam && segmentParam.value !== null
? {
...parentParams,
[segmentParam.param]: segmentParam.value,
}
: parentParams
const actualSegment = segmentParam
? [segmentParam.param, segmentParam.value]
? [segmentParam.param, segmentParam.treeValue]
: segment

// This happens outside of rendering in order to eagerly kick off data fetching for layouts / the page further down
Expand All @@ -539,7 +539,7 @@ export async function renderToHTML(
const childProp: ChildProp = {
current: <ChildComponent />,
segment: childSegmentParam
? [childSegmentParam.param, childSegmentParam.value]
? [childSegmentParam.param, childSegmentParam.treeValue]
: parallelRoutes[currentValue][0],
}

Expand Down Expand Up @@ -669,25 +669,26 @@ export async function renderToHTML(
// TODO: throw on invalid flightRouterState
const walkTreeWithFlightRouterState = (
treeToFilter: LoaderTree,
parentParams: { [key: string]: any },
parentParams: { [key: string]: string | string[] },
flightRouterState?: FlightRouterState,
parentRendered?: boolean
): FlightDataPath => {
const [segment, parallelRoutes] = treeToFilter
const parallelRoutesKeys = Object.keys(parallelRoutes)

const segmentParam = getDynamicParamFromSegment(segment)
const currentParams =
// Handle null case where dynamic param is optional
segmentParam && segmentParam.value !== null
? {
...parentParams,
[segmentParam.param]: segmentParam.value,
}
: parentParams
const actualSegment: Segment = segmentParam
? [segmentParam.param, segmentParam.value]
? [segmentParam.param, segmentParam.treeValue]
: segment

const currentParams = segmentParam
? {
...parentParams,
[segmentParam.param]: segmentParam.value,
}
: parentParams

const renderComponentsOnThisLevel =
!flightRouterState ||
!matchSegment(actualSegment, flightRouterState[0]) ||
Expand Down

0 comments on commit 050d13e

Please sign in to comment.