Skip to content

Commit 3869ee9

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 16573a0 commit 3869ee9

File tree

12 files changed

+68
-26
lines changed

12 files changed

+68
-26
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: 6 additions & 5 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
) {
3940
// this is used to pass query information in rewrites
4041
// but should not be exposed in final query
@@ -60,7 +61,7 @@ function filterInternalQuery(
6061

6162
export function normalizeCdnUrl(
6263
req: BaseNextRequest | IncomingMessage,
63-
paramKeys: string[]
64+
paramKeys: readonly string[]
6465
) {
6566
// make sure to normalize req.url from CDNs to strip dynamic and rewrite
6667
// params from the query which are added during routing
@@ -83,7 +84,7 @@ export function interpolateDynamicPath(
8384
) {
8485
if (!defaultRouteRegex) return pathname
8586

86-
for (const param of Object.keys(defaultRouteRegex.groups)) {
87+
for (const param of getRouteParamKeys(defaultRouteRegex.groups)) {
8788
const { optional, repeat } = defaultRouteRegex.groups[param]
8889
let builtParam = `[${repeat ? '...' : ''}${param}]`
8990

@@ -119,7 +120,7 @@ export function normalizeDynamicRouteParams(
119120
let hasValidParams = true
120121
let params: ParsedUrlQuery = {}
121122

122-
for (const key of Object.keys(defaultRouteRegex.groups)) {
123+
for (const key of getRouteParamKeys(defaultRouteRegex.groups)) {
123124
let value: string | string[] | undefined = query[key]
124125

125126
if (typeof value === 'string') {
@@ -396,7 +397,7 @@ export function getServerUtils({
396397

397398
// Use all the named route keys.
398399
const result = {} as RegExpExecArray
399-
for (const keyName of Object.keys(routeKeys)) {
400+
for (const keyName of getRouteParamKeys(groups)) {
400401
const paramName = routeKeys[keyName]
401402

402403
// 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))
190+
normalizeCdnUrl(req, getRouteParamKeys(routeRegex.groups))
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)