Skip to content

Commit

Permalink
Reapply "fix: handle optional catchalls correctly" (#75442) (#75450)
Browse files Browse the repository at this point in the history
This is a reapplication of the [original
fix](#75377) for optional catchall
routes but narrowed to only when the parameters weren't already valid.
  • Loading branch information
wyattjoh authored Jan 30, 2025
1 parent 8202690 commit 28ac628
Show file tree
Hide file tree
Showing 9 changed files with 251 additions and 83 deletions.
26 changes: 21 additions & 5 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1182,7 +1182,8 @@ export default abstract class Server<
let params: ParsedUrlQuery | false = {}

let paramsResult = utils.normalizeDynamicRouteParams(
parsedUrl.query
parsedUrl.query,
false
)

// for prerendered ISR paths we attempt parsing the route
Expand All @@ -1196,7 +1197,7 @@ export default abstract class Server<
let matcherParams = utils.dynamicRouteMatcher?.(normalizedUrlPath)

if (matcherParams) {
utils.normalizeDynamicRouteParams(matcherParams)
utils.normalizeDynamicRouteParams(matcherParams, false)
Object.assign(paramsResult.params, matcherParams)
paramsResult.hasValidParams = true
}
Expand All @@ -1218,8 +1219,10 @@ export default abstract class Server<
let matcherParams = utils.dynamicRouteMatcher?.(matchedPath)

if (matcherParams) {
const curParamsResult =
utils.normalizeDynamicRouteParams(matcherParams)
const curParamsResult = utils.normalizeDynamicRouteParams(
matcherParams,
false
)

if (curParamsResult.hasValidParams) {
Object.assign(params, matcherParams)
Expand Down Expand Up @@ -1254,6 +1257,19 @@ export default abstract class Server<
}
}

// Try to parse the params from the query if we couldn't parse them
// from the route matches but ignore missing optional params.
if (!paramsResult.hasValidParams) {
paramsResult = utils.normalizeDynamicRouteParams(
parsedUrl.query,
true
)

if (paramsResult.hasValidParams) {
params = paramsResult.params
}
}

// handle the actual dynamic route name being requested
if (
utils.defaultRouteMatches &&
Expand All @@ -1276,7 +1292,7 @@ export default abstract class Server<
}

if (pageIsDynamic || didRewrite) {
utils.normalizeVercelUrl(req, true, [
utils.normalizeVercelUrl(req, [
...rewriteParamKeys,
...Object.keys(utils.defaultRouteRegex?.groups || {}),
])
Expand Down
125 changes: 59 additions & 66 deletions packages/next/src/server/server-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,36 +24,34 @@ import { normalizeNextQueryParam } from './web/utils'

export function normalizeVercelUrl(
req: BaseNextRequest,
trustQuery: boolean,
paramKeys?: string[],
pageIsDynamic?: boolean,
defaultRouteRegex?: ReturnType<typeof getNamedRouteRegex> | undefined
paramKeys: string[],
defaultRouteRegex: ReturnType<typeof getNamedRouteRegex> | undefined
) {
if (!defaultRouteRegex) return

// make sure to normalize req.url on Vercel to strip dynamic params
// from the query which are added during routing
if (pageIsDynamic && trustQuery && defaultRouteRegex) {
const _parsedUrl = parseUrl(req.url!, true)
delete (_parsedUrl as any).search

for (const key of Object.keys(_parsedUrl.query)) {
const isNextQueryPrefix =
key !== NEXT_QUERY_PARAM_PREFIX &&
key.startsWith(NEXT_QUERY_PARAM_PREFIX)

const isNextInterceptionMarkerPrefix =
key !== NEXT_INTERCEPTION_MARKER_PREFIX &&
key.startsWith(NEXT_INTERCEPTION_MARKER_PREFIX)

if (
isNextQueryPrefix ||
isNextInterceptionMarkerPrefix ||
(paramKeys || Object.keys(defaultRouteRegex.groups)).includes(key)
) {
delete _parsedUrl.query[key]
}
const _parsedUrl = parseUrl(req.url!, true)
delete (_parsedUrl as any).search

for (const key of Object.keys(_parsedUrl.query)) {
const isNextQueryPrefix =
key !== NEXT_QUERY_PARAM_PREFIX && key.startsWith(NEXT_QUERY_PARAM_PREFIX)

const isNextInterceptionMarkerPrefix =
key !== NEXT_INTERCEPTION_MARKER_PREFIX &&
key.startsWith(NEXT_INTERCEPTION_MARKER_PREFIX)

if (
isNextQueryPrefix ||
isNextInterceptionMarkerPrefix ||
(paramKeys || Object.keys(defaultRouteRegex.groups)).includes(key)
) {
delete _parsedUrl.query[key]
}
req.url = formatUrl(_parsedUrl)
}

req.url = formatUrl(_parsedUrl)
}

export function interpolateDynamicPath(
Expand Down Expand Up @@ -89,27 +87,21 @@ export function interpolateDynamicPath(
}

export function normalizeDynamicRouteParams(
params: ParsedUrlQuery,
ignoreOptional?: boolean,
defaultRouteRegex?: ReturnType<typeof getNamedRouteRegex> | undefined,
defaultRouteMatches?: ParsedUrlQuery | undefined
query: ParsedUrlQuery,
defaultRouteRegex: ReturnType<typeof getNamedRouteRegex>,
defaultRouteMatches: ParsedUrlQuery,
ignoreMissingOptional: boolean
) {
let hasValidParams = true
if (!defaultRouteRegex) return { params, hasValidParams: false }
let params: ParsedUrlQuery = {}

params = Object.keys(defaultRouteRegex.groups).reduce((prev, key) => {
let value: string | string[] | undefined = params[key]
for (const key of Object.keys(defaultRouteRegex.groups)) {
let value: string | string[] | undefined = query[key]

if (typeof value === 'string') {
value = normalizeRscURL(value)
}
if (Array.isArray(value)) {
value = value.map((val) => {
if (typeof val === 'string') {
val = normalizeRscURL(val)
}
return val
})
} else if (Array.isArray(value)) {
value = value.map(normalizeRscURL)
}

// if the value matches the default value we can't rely
Expand All @@ -128,9 +120,9 @@ export function normalizeDynamicRouteParams(

if (
isDefaultValue ||
(typeof value === 'undefined' && !(isOptional && ignoreOptional))
(typeof value === 'undefined' && !(isOptional && ignoreMissingOptional))
) {
hasValidParams = false
return { params: {}, hasValidParams: false }
}

// non-provided optional values should be undefined so normalize
Expand All @@ -145,7 +137,7 @@ export function normalizeDynamicRouteParams(
(value[0] === 'index' || value[0] === `[[...${key}]]`)))
) {
value = undefined
delete params[key]
delete query[key]
}

// query values from the proxy aren't already split into arrays
Expand All @@ -159,10 +151,9 @@ export function normalizeDynamicRouteParams(
}

if (value) {
prev[key] = value
params[key] = value
}
return prev
}, {} as ParsedUrlQuery)
}

return {
params,
Expand Down Expand Up @@ -375,28 +366,30 @@ export function getUtils({
dynamicRouteMatcher,
defaultRouteMatches,
getParamsFromRouteMatches,
/**
* Normalize dynamic route params.
*
* @param query - The query params to normalize.
* @param ignoreMissingOptional - Whether to ignore missing optional params.
* @returns The normalized params and whether they are valid.
*/
normalizeDynamicRouteParams: (
params: ParsedUrlQuery,
ignoreOptional?: boolean
) =>
normalizeDynamicRouteParams(
params,
ignoreOptional,
query: ParsedUrlQuery,
ignoreMissingOptional: boolean
) => {
if (!defaultRouteRegex || !defaultRouteMatches) {
return { params: {}, hasValidParams: false }
}

return normalizeDynamicRouteParams(
query,
defaultRouteRegex,
defaultRouteMatches
),
normalizeVercelUrl: (
req: BaseNextRequest,
trustQuery: boolean,
paramKeys?: string[]
) =>
normalizeVercelUrl(
req,
trustQuery,
paramKeys,
pageIsDynamic,
defaultRouteRegex
),
defaultRouteMatches,
ignoreMissingOptional
)
},
normalizeVercelUrl: (req: BaseNextRequest, paramKeys: string[]) =>
normalizeVercelUrl(req, paramKeys, defaultRouteRegex),
interpolateDynamicPath: (
pathname: string,
params: Record<string, undefined | string | string[]>
Expand Down
12 changes: 3 additions & 9 deletions packages/next/src/server/web-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,9 @@ export default class NextWebServer extends BaseServer<
) as NextParsedUrlQuery
const paramsResult = normalizeDynamicRouteParams(
query,
false,
routeRegex,
defaultRouteMatches
defaultRouteMatches,
false
)
const normalizedParams = paramsResult.hasValidParams
? paramsResult.params
Expand All @@ -186,13 +186,7 @@ export default class NextWebServer extends BaseServer<
normalizedParams,
routeRegex
)
normalizeVercelUrl(
req,
true,
Object.keys(routeRegex.routeKeys),
true,
routeRegex
)
normalizeVercelUrl(req, Object.keys(routeRegex.routeKeys), routeRegex)
}
}

Expand Down
3 changes: 2 additions & 1 deletion packages/next/src/server/web/edge-route-module-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ export class EdgeRouteModuleWrapper {
})

const { params } = utils.normalizeDynamicRouteParams(
searchParamsToUrlQuery(request.nextUrl.searchParams)
searchParamsToUrlQuery(request.nextUrl.searchParams),
false
)

const waitUntil = evt.waitUntil.bind(evt)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { useRouter } from 'next/router'

export const getStaticProps = ({ params }) => {
return {
props: {
hello: 'world',
params: params || null,
random: Math.random(),
},
}
}

export const getStaticPaths = () => {
return {
paths: ['/partial-catch-all/hello.com'],
fallback: true,
}
}

export default function Page(props) {
const router = useRouter()
return (
<>
<p id="catch-all">partial optional catch-all page</p>
<p id="router">{JSON.stringify(router)}</p>
<p id="props">{JSON.stringify(props)}</p>
</>
)
}
Loading

0 comments on commit 28ac628

Please sign in to comment.