Skip to content

Commit a376b03

Browse files
committed
refactor(router): use getRouteParamKeys for consistent parameter ordering
This refactor introduces a new getRouteParamKeys utility function that ensures route parameters are always accessed in the correct order based on their position in the route pattern. Previously, Object.keys() was used to iterate over route parameters, which could lead to inconsistent ordering since JavaScript object key order is not guaranteed for numeric string keys. Changes: - Add getRouteParamKeys utility that sorts parameters by their position - Replace Object.keys(groups) with getRouteParamKeys(groups) in: - server-utils.ts (normalizeDynamicRouteParams, interpolateDynamicPath, filterInternalQuery) - base-server.ts (normalizeCdnUrl parameter extraction) - Update omit() utility to accept readonly arrays for better type safety - Add comprehensive tests for getRouteParamKeys This ensures deterministic behavior when processing route parameters, which is critical for proper URL interpolation and parameter extraction.
1 parent 0f2c152 commit a376b03

File tree

12 files changed

+60
-27
lines changed

12 files changed

+60
-27
lines changed

packages/next/src/build/static-paths/app.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import path from 'node:path'
77
import { AfterRunner } from '../../server/after/run-with-after'
88
import { createWorkStore } from '../../server/async-storage/work-store'
99
import { FallbackMode } from '../../lib/fallback'
10-
import { getRouteMatcher } from '../../shared/lib/router/utils/route-matcher'
10+
import { getRouteParamKeys } from '../../shared/lib/router/utils/route-param-keys'
1111
import {
1212
getRouteRegex,
1313
type RouteRegex,
@@ -471,7 +471,7 @@ export async function buildAppStaticPaths({
471471
})
472472

473473
const regex = getRouteRegex(page)
474-
const routeParamKeys = Object.keys(getRouteMatcher(regex)(page) || {})
474+
const routeParamKeys = getRouteParamKeys(regex.groups)
475475

476476
const afterRunner = new AfterRunner()
477477

packages/next/src/build/static-paths/pages.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { removeTrailingSlash } from '../../shared/lib/router/utils/remove-traili
88
import { getRouteMatcher } from '../../shared/lib/router/utils/route-matcher'
99
import { getRouteRegex } from '../../shared/lib/router/utils/route-regex'
1010
import { encodeParam, normalizePathname } from './utils'
11+
import { getRouteParamKeys } from '../../shared/lib/router/utils/route-param-keys'
1112

1213
export async function buildPagesStaticPaths({
1314
page,
@@ -27,7 +28,7 @@ export async function buildPagesStaticPaths({
2728
const _routeMatcher = getRouteMatcher(_routeRegex)
2829

2930
// Get the default list of allowed params.
30-
const routeParameterKeys = Object.keys(_routeMatcher(page))
31+
const routeParameterKeys = getRouteParamKeys(_routeRegex.groups)
3132
const staticPathsResult = await getStaticPaths({
3233
// We create a copy here to avoid having the types of `getStaticPaths`
3334
// change. This ensures that users can't mutate this array and have it

packages/next/src/server/base-server.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ import { denormalizePagePath } from '../shared/lib/page-path/denormalize-page-pa
8585
import * as Log from '../build/output/log'
8686
import { getPreviouslyRevalidatedTags, getServerUtils } from './server-utils'
8787
import isError, { getProperError } from '../lib/is-error'
88+
import { getRouteParamKeys } from '../shared/lib/router/utils/route-param-keys'
8889
import {
8990
addRequestMeta,
9091
getRequestMeta,
@@ -1396,7 +1397,9 @@ export default abstract class Server<
13961397
if (pageIsDynamic || didRewrite) {
13971398
utils.normalizeCdnUrl(req, [
13981399
...rewriteParamKeys,
1399-
...Object.keys(utils.defaultRouteRegex?.groups || {}),
1400+
...(utils.defaultRouteRegex
1401+
? getRouteParamKeys(utils.defaultRouteRegex.groups)
1402+
: []),
14001403
])
14011404
}
14021405
// Remove the route `params` keys from `parsedUrl.query` if they are

packages/next/src/server/request/fallback-params.ts

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,15 @@
1-
import { getRouteMatcher } from '../../shared/lib/router/utils/route-matcher'
1+
import { getRouteParamKeys } from '../../shared/lib/router/utils/route-param-keys'
22
import { getRouteRegex } from '../../shared/lib/router/utils/route-regex'
33

44
export type FallbackRouteParams = ReadonlyMap<string, string>
55

6-
function getParamKeys(page: string) {
7-
const pattern = getRouteRegex(page)
8-
const matcher = getRouteMatcher(pattern)
9-
10-
// Get the default list of allowed params.
11-
return Object.keys(matcher(page))
12-
}
13-
146
export function getFallbackRouteParams(
157
pageOrKeys: string | readonly string[]
168
): FallbackRouteParams | null {
179
let keys: readonly string[]
1810
if (typeof pageOrKeys === 'string') {
19-
keys = getParamKeys(pageOrKeys)
11+
const { groups } = getRouteRegex(pageOrKeys)
12+
keys = getRouteParamKeys(groups)
2013
} else {
2114
keys = pageOrKeys
2215
}

packages/next/src/server/server-utils.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,11 @@ import { parseAndValidateFlightRouterState } from './app-render/parse-and-valida
3131
import { isInterceptionRouteRewrite } from '../lib/generate-interception-routes-rewrites'
3232
import { NEXT_ROUTER_STATE_TREE_HEADER } from '../client/components/app-router-headers'
3333
import { getSelectedParams } from '../client/components/router-reducer/compute-changed-path'
34+
import { getRouteParamKeys } from '../shared/lib/router/utils/route-param-keys'
3435

3536
function filterInternalQuery(
3637
query: Record<string, undefined | string | string[]>,
37-
paramKeys: string[],
38+
paramKeys: readonly string[],
3839
defaultRouteRegex: ReturnType<typeof getNamedRouteRegex> | undefined
3940
) {
4041
// this is used to pass query information in rewrites
@@ -53,7 +54,8 @@ function filterInternalQuery(
5354
isNextQueryPrefix ||
5455
isNextInterceptionMarkerPrefix ||
5556
paramKeys.includes(key) ||
56-
(defaultRouteRegex && Object.keys(defaultRouteRegex.groups).includes(key))
57+
(defaultRouteRegex &&
58+
getRouteParamKeys(defaultRouteRegex.groups).includes(key))
5759
) {
5860
delete query[key]
5961
}
@@ -62,7 +64,7 @@ function filterInternalQuery(
6264

6365
export function normalizeCdnUrl(
6466
req: BaseNextRequest | IncomingMessage,
65-
paramKeys: string[],
67+
paramKeys: readonly string[],
6668
defaultRouteRegex: ReturnType<typeof getNamedRouteRegex> | undefined
6769
) {
6870
// make sure to normalize req.url from CDNs to strip dynamic and rewrite
@@ -86,7 +88,7 @@ export function interpolateDynamicPath(
8688
) {
8789
if (!defaultRouteRegex) return pathname
8890

89-
for (const param of Object.keys(defaultRouteRegex.groups)) {
91+
for (const param of getRouteParamKeys(defaultRouteRegex.groups)) {
9092
const { optional, repeat } = defaultRouteRegex.groups[param]
9193
let builtParam = `[${repeat ? '...' : ''}${param}]`
9294

@@ -122,7 +124,7 @@ export function normalizeDynamicRouteParams(
122124
let hasValidParams = true
123125
let params: ParsedUrlQuery = {}
124126

125-
for (const key of Object.keys(defaultRouteRegex.groups)) {
127+
for (const key of getRouteParamKeys(defaultRouteRegex.groups)) {
126128
let value: string | string[] | undefined = query[key]
127129

128130
if (typeof value === 'string') {
@@ -399,7 +401,7 @@ export function getServerUtils({
399401

400402
// Use all the named route keys.
401403
const result = {} as RegExpExecArray
402-
for (const keyName of Object.keys(routeKeys)) {
404+
for (const keyName of getRouteParamKeys(groups)) {
403405
const paramName = routeKeys[keyName]
404406

405407
// If this param name is not a valid parameter name, then skip it.

packages/next/src/server/web-server.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import { getEdgeInstrumentationModule } from './web/globals'
3535
import type { ServerOnInstrumentationRequestError } from './app-render/types'
3636
import { getEdgePreviewProps } from './web/get-edge-preview-props'
3737
import { NoFallbackError } from '../shared/lib/no-fallback-error.external'
38+
import { getRouteParamKeys } from '../shared/lib/router/utils/route-param-keys'
3839

3940
interface WebServerOptions extends Options {
4041
buildId: string
@@ -186,7 +187,7 @@ export default class NextWebServer extends BaseServer<
186187
normalizedParams,
187188
routeRegex
188189
)
189-
normalizeCdnUrl(req, Object.keys(routeRegex.routeKeys), routeRegex)
190+
normalizeCdnUrl(req, getRouteParamKeys(routeRegex.groups), routeRegex)
190191
}
191192
}
192193

packages/next/src/shared/lib/router/adapters.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { PathnameContext } from '../hooks-client-context.shared-runtime'
77
import { isDynamicRoute } from './utils'
88
import { asPathToSearchParams } from './utils/as-path-to-search-params'
99
import { getRouteRegex } from './utils/route-regex'
10+
import { getRouteParamKeys } from './utils/route-param-keys'
1011

1112
/** It adapts a Pages Router (`NextRouter`) to the App Router Instance. */
1213
export function adaptForAppRouterInstance(
@@ -59,7 +60,7 @@ export function adaptForPathParams(
5960
}
6061
const pathParams: Params = {}
6162
const routeRegex = getRouteRegex(router.pathname)
62-
const keys = Object.keys(routeRegex.groups)
63+
const keys = getRouteParamKeys(routeRegex.groups)
6364
for (const key of keys) {
6465
pathParams[key] = router.query[key]!
6566
}

packages/next/src/shared/lib/router/router.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { isDynamicRoute } from './utils/is-dynamic'
2424
import { parseRelativeUrl } from './utils/parse-relative-url'
2525
import { getRouteMatcher } from './utils/route-matcher'
2626
import { getRouteRegex } from './utils/route-regex'
27+
import { getRouteParamKeys } from './utils/route-param-keys'
2728
import { formatWithValidation } from './utils/format-url'
2829
import { detectDomainLocale } from '../../../client/detect-domain-locale'
2930
import { parsePath } from './utils/parse-path'
@@ -1504,7 +1505,7 @@ export default class Router implements BaseRouter {
15041505
: ({} as { result: undefined; params: undefined })
15051506

15061507
if (!routeMatch || (shouldInterpolate && !interpolatedAs.result)) {
1507-
const missingParams = Object.keys(routeRegex.groups).filter(
1508+
const missingParams = getRouteParamKeys(routeRegex.groups).filter(
15081509
(param) => !query[param] && !routeRegex.groups[param].optional
15091510
)
15101511

packages/next/src/shared/lib/router/utils/interpolate-as.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { ParsedUrlQuery } from 'querystring'
22

33
import { getRouteMatcher } from './route-matcher'
44
import { getRouteRegex } from './route-regex'
5+
import { getRouteParamKeys } from './route-param-keys'
56

67
export function interpolateAs(
78
route: string,
@@ -11,7 +12,6 @@ export function interpolateAs(
1112
let interpolatedRoute = ''
1213

1314
const dynamicRegex = getRouteRegex(route)
14-
const dynamicGroups = dynamicRegex.groups
1515
const dynamicMatches =
1616
// Try to match the dynamic route against the asPath
1717
(asPathname !== route ? getRouteMatcher(dynamicRegex)(asPathname) : '') ||
@@ -20,12 +20,12 @@ export function interpolateAs(
2020
query
2121

2222
interpolatedRoute = route
23-
const params = Object.keys(dynamicGroups)
23+
const params = getRouteParamKeys(dynamicRegex.groups)
2424

2525
if (
2626
!params.every((param) => {
2727
let value = dynamicMatches[param] || ''
28-
const { repeat, optional } = dynamicGroups[param]
28+
const { repeat, optional } = dynamicRegex.groups[param]
2929

3030
// support single-level catch-all
3131
// TODO: more robust handling for user-error (passing `/`)

packages/next/src/shared/lib/router/utils/omit.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
export function omit<T extends { [key: string]: unknown }, K extends keyof T>(
22
object: T,
3-
keys: K[]
3+
keys: readonly K[]
44
): Omit<T, K> {
55
const omitted: { [key: string]: unknown } = {}
66
Object.keys(object).forEach((key) => {

0 commit comments

Comments
 (0)