Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update search params/route params handling on deploy #47930

Merged
merged 8 commits into from Apr 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 9 additions & 6 deletions packages/next/src/build/index.ts
Expand Up @@ -190,7 +190,7 @@ async function generateClientSsgManifest(
}

function pageToRoute(page: string) {
const routeRegex = getNamedRouteRegex(page)
const routeRegex = getNamedRouteRegex(page, true)
return {
page,
regex: normalizeRouteRegex(routeRegex.re.source),
Expand Down Expand Up @@ -1975,7 +1975,8 @@ export default async function build(

if (isDynamicRoute(page)) {
const routeRegex = getNamedRouteRegex(
dataRoute.replace(/\.json$/, '')
dataRoute.replace(/\.json$/, ''),
true
)

dataRouteRegex = normalizeRouteRegex(
Expand Down Expand Up @@ -2392,7 +2393,7 @@ export default async function build(
// dynamicParams for non-static paths?
finalDynamicRoutes[page] = {
routeRegex: normalizeRouteRegex(
getNamedRouteRegex(page).re.source
getNamedRouteRegex(page, false).re.source
),
dataRoute,
// if dynamicParams are enabled treat as fallback:
Expand All @@ -2404,7 +2405,8 @@ export default async function build(
? null
: normalizeRouteRegex(
getNamedRouteRegex(
dataRoute.replace(/\.rsc$/, '')
dataRoute.replace(/\.rsc$/, ''),
false
).re.source.replace(/\(\?:\\\/\)\?\$$/, '\\.rsc$')
),
}
Expand Down Expand Up @@ -2769,7 +2771,7 @@ export default async function build(

finalDynamicRoutes[tbdRoute] = {
routeRegex: normalizeRouteRegex(
getNamedRouteRegex(tbdRoute).re.source
getNamedRouteRegex(tbdRoute, false).re.source
),
dataRoute,
fallback: ssgBlockingFallbackPages.has(tbdRoute)
Expand All @@ -2779,7 +2781,8 @@ export default async function build(
: false,
dataRouteRegex: normalizeRouteRegex(
getNamedRouteRegex(
dataRoute.replace(/\.json$/, '')
dataRoute.replace(/\.json$/, ''),
false
).re.source.replace(/\(\?:\\\/\)\?\$$/, '\\.json$')
),
}
Expand Down
Expand Up @@ -59,7 +59,7 @@ async function nextMetadataImageLoader(this: any, content: Buffer) {
const exportedImageData = { ...exported }
export default (props) => {
const pathname = ${JSON.stringify(route)}
const routeRegex = getNamedRouteRegex(pathname)
const routeRegex = getNamedRouteRegex(pathname, false)
const route = interpolateDynamicPath(pathname, props.params, routeRegex)

const imageData = {
Expand Down Expand Up @@ -111,7 +111,7 @@ async function nextMetadataImageLoader(this: any, content: Buffer) {

export default (props) => {
const pathname = ${JSON.stringify(route)}
const routeRegex = getNamedRouteRegex(pathname)
const routeRegex = getNamedRouteRegex(pathname, false)
const route = interpolateDynamicPath(pathname, props.params, routeRegex)

const imageData = ${JSON.stringify(imageData)};
Expand Down
2 changes: 2 additions & 0 deletions packages/next/src/lib/constants.ts
@@ -1,5 +1,7 @@
import type { ServerRuntime } from '../../types'

export const NEXT_QUERY_PARAM_PREFIX = 'nextParam'

// in seconds
export const CACHE_ONE_YEAR = 31536000

Expand Down
23 changes: 22 additions & 1 deletion packages/next/src/server/base-server.ts
Expand Up @@ -101,6 +101,7 @@ import { RouteKind } from './future/route-kind'
import { handleInternalServerErrorResponse } from './future/route-modules/helpers/response-handlers'
import { parseNextReferrerFromHeaders } from './lib/parse-next-referrer'
import { fromNodeHeaders, toNodeHeaders } from './web/utils'
import { NEXT_QUERY_PARAM_PREFIX } from '../lib/constants'

export type FindComponentsResult = {
components: LoadComponentsReturnType
Expand Down Expand Up @@ -783,6 +784,24 @@ export default abstract class Server<ServerOptions extends Options = Options> {
addRequestMeta(req, '_nextRewroteUrl', parsedUrl.pathname!)
addRequestMeta(req, '_nextDidRewrite', true)
}
const routeParamKeys = new Set<string>()

for (const key of Object.keys(parsedUrl.query)) {
const value = parsedUrl.query[key]

if (
key !== NEXT_QUERY_PARAM_PREFIX &&
key.startsWith(NEXT_QUERY_PARAM_PREFIX)
) {
const normalizedKey = key.substring(
NEXT_QUERY_PARAM_PREFIX.length
)
parsedUrl.query[normalizedKey] = value

routeParamKeys.add(normalizedKey)
delete parsedUrl.query[key]
}
}

// interpolate dynamic params and normalize URL if needed
if (pageIsDynamic) {
Expand Down Expand Up @@ -861,7 +880,6 @@ export default abstract class Server<ServerOptions extends Options = Options> {
matchedPath = utils.interpolateDynamicPath(srcPathname, params)
req.url = utils.interpolateDynamicPath(req.url!, params)
}
Object.assign(parsedUrl.query, params)
}

if (pageIsDynamic || didRewrite) {
Expand All @@ -870,6 +888,9 @@ export default abstract class Server<ServerOptions extends Options = Options> {
...Object.keys(utils.defaultRouteRegex?.groups || {}),
])
}
for (const key of routeParamKeys) {
delete parsedUrl.query[key]
}
parsedUrl.pathname = `${this.nextConfig.basePath || ''}${
matchedPath === '/' && this.nextConfig.basePath ? '' : matchedPath
}`
Expand Down
13 changes: 10 additions & 3 deletions packages/next/src/server/server-utils.ts
Expand Up @@ -23,6 +23,7 @@ import { TEMPORARY_REDIRECT_STATUS } from '../shared/lib/constants'
import { addRequestMeta } from './request-meta'
import { removeTrailingSlash } from '../shared/lib/router/utils/remove-trailing-slash'
import { normalizeRscPath } from '../shared/lib/router/utils/app-paths'
import { NEXT_QUERY_PARAM_PREFIX } from '../lib/constants'

export function normalizeVercelUrl(
req: BaseNextRequest | IncomingMessage,
Expand All @@ -37,8 +38,14 @@ export function normalizeVercelUrl(
const _parsedUrl = parseUrl(req.url!, true)
delete (_parsedUrl as any).search

for (const param of paramKeys || Object.keys(defaultRouteRegex.groups)) {
delete _parsedUrl.query[param]
for (const key of Object.keys(_parsedUrl.query)) {
if (
(key !== NEXT_QUERY_PARAM_PREFIX &&
key.startsWith(NEXT_QUERY_PARAM_PREFIX)) ||
(paramKeys || Object.keys(defaultRouteRegex.groups)).includes(key)
) {
delete _parsedUrl.query[key]
}
}
req.url = formatUrl(_parsedUrl)
}
Expand Down Expand Up @@ -107,7 +114,7 @@ export function getUtils({
let defaultRouteMatches: ParsedUrlQuery | undefined

if (pageIsDynamic) {
defaultRouteRegex = getNamedRouteRegex(page)
defaultRouteRegex = getNamedRouteRegex(page, false)
dynamicRouteMatcher = getRouteMatcher(defaultRouteRegex)
defaultRouteMatches = dynamicRouteMatcher(page) as ParsedUrlQuery
}
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/server/web-server.ts
Expand Up @@ -290,7 +290,7 @@ export default class NextWebServer extends BaseServer<WebServerOptions> {
pathname = this.serverOptions.webServerConfig.page

if (isDynamicRoute(pathname)) {
const routeRegex = getNamedRouteRegex(pathname)
const routeRegex = getNamedRouteRegex(pathname, false)
pathname = interpolateDynamicPath(pathname, query, routeRegex)
normalizeVercelUrl(
req,
Expand Down
29 changes: 23 additions & 6 deletions packages/next/src/shared/lib/router/utils/route-regex.ts
@@ -1,6 +1,8 @@
import { escapeStringRegexp } from '../../escape-regexp'
import { removeTrailingSlash } from './remove-trailing-slash'

const NEXT_QUERY_PARAM_PREFIX = 'nextParam'

export interface Group {
pos: number
repeat: boolean
Expand Down Expand Up @@ -88,7 +90,7 @@ function buildGetSafeRouteKey() {
}
}

function getNamedParametrizedRoute(route: string) {
function getNamedParametrizedRoute(route: string, prefixRouteKeys: boolean) {
const segments = removeTrailingSlash(route).slice(1).split('/')
const getSafeRouteKey = buildGetSafeRouteKey()
const routeKeys: { [named: string]: string } = {}
Expand All @@ -115,7 +117,13 @@ function getNamedParametrizedRoute(route: string) {
cleanedKey = getSafeRouteKey()
}

routeKeys[cleanedKey] = key
if (prefixRouteKeys) {
cleanedKey = `${NEXT_QUERY_PARAM_PREFIX}${cleanedKey}`
routeKeys[cleanedKey] = `${NEXT_QUERY_PARAM_PREFIX}${key}`
} else {
routeKeys[cleanedKey] = `${key}`
}

return repeat
? optional
? `(?:/(?<${cleanedKey}>.+?))?`
Expand All @@ -133,10 +141,16 @@ function getNamedParametrizedRoute(route: string) {
/**
* This function extends `getRouteRegex` generating also a named regexp where
* each group is named along with a routeKeys object that indexes the assigned
* named group with its corresponding key.
* named group with its corresponding key. When the routeKeys need to be
* prefixed to uniquely identify internally the "prefixRouteKey" arg should
* be "true" currently this is only the case when creating the routes-manifest
* during the build
*/
export function getNamedRouteRegex(normalizedRoute: string) {
const result = getNamedParametrizedRoute(normalizedRoute)
export function getNamedRouteRegex(
normalizedRoute: string,
prefixRouteKey: boolean
) {
const result = getNamedParametrizedRoute(normalizedRoute, prefixRouteKey)
return {
...getRouteRegex(normalizedRoute),
namedRegex: `^${result.namedParameterizedRoute}(?:/)?$`,
Expand All @@ -163,7 +177,10 @@ export function getNamedMiddlewareRegex(
}
}

const { namedParameterizedRoute } = getNamedParametrizedRoute(normalizedRoute)
const { namedParameterizedRoute } = getNamedParametrizedRoute(
normalizedRoute,
false
)
let catchAllGroupedRegex = catchAll ? '(?:(/.*)?)' : ''
return {
namedRegex: `^${namedParameterizedRoute}${catchAllGroupedRegex}$`,
Expand Down
@@ -1,5 +1,7 @@
'use client'

import { useSearchParams } from 'next/navigation'

export default function IdPage({ children, params }) {
return (
<>
Expand All @@ -8,6 +10,10 @@ export default function IdPage({ children, params }) {
<span id="id-page-params">{JSON.stringify(params)}</span>
</p>
{children}

<p id="search-params">
{JSON.stringify(Object.fromEntries(useSearchParams()))}
</p>
</>
)
}
4 changes: 3 additions & 1 deletion test/e2e/app-dir/app/app/dynamic/[category]/[id]/page.js
@@ -1,11 +1,13 @@
export default function IdPage({ children, params }) {
export default function IdPage({ children, params, searchParams }) {
return (
<>
<p>
Id Page. Params:{' '}
<span id="id-page-params">{JSON.stringify(params)}</span>
</p>
{children}

<p id="search-params">{JSON.stringify(searchParams)}</p>
</>
)
}
29 changes: 29 additions & 0 deletions test/e2e/app-dir/app/index.test.ts
Expand Up @@ -10,6 +10,35 @@ createNextDescribe(
files: __dirname,
},
({ next, isNextDev: isDev, isNextStart, isNextDeploy }) => {
it('should have correct searchParams and params (server)', async () => {
const html = await next.render('/dynamic/category-1/id-2?query1=value2')
const $ = cheerio.load(html)

expect(JSON.parse($('#id-page-params').text())).toEqual({
category: 'category-1',
id: 'id-2',
})
expect(JSON.parse($('#search-params').text())).toEqual({
query1: 'value2',
})
})

it('should have correct searchParams and params (client)', async () => {
const browser = await next.browser(
'/dynamic-client/category-1/id-2?query1=value2'
)
const html = await browser.eval('document.documentElement.innerHTML')
const $ = cheerio.load(html)

expect(JSON.parse($('#id-page-params').text())).toEqual({
category: 'category-1',
id: 'id-2',
})
expect(JSON.parse($('#search-params').text())).toEqual({
query1: 'value2',
})
})

if (!isDev) {
it('should successfully detect app route during prefetch', async () => {
const browser = await next.browser('/')
Expand Down
Expand Up @@ -5,6 +5,8 @@ createNextDescribe(
'parallel-routes-and-interception',
{
files: __dirname,
// TODO: remove after deployment handling is updated
skipDeployment: true,
},
({ next }) => {
describe('parallel routes', () => {
Expand Down
24 changes: 0 additions & 24 deletions test/e2e/app-dir/parallel-routes-and-interception/tsconfig.json

This file was deleted.

4 changes: 2 additions & 2 deletions test/e2e/edge-render-getserversideprops/index.test.ts
Expand Up @@ -140,10 +140,10 @@ describe('edge-render-getserversideprops', () => {
),
namedDataRouteRegex: `^/_next/data/${escapeStringRegexp(
next.buildId
)}/(?<id>[^/]+?)\\.json$`,
)}/(?<nextParamid>[^/]+?)\\.json$`,
page: '/[id]',
routeKeys: {
id: 'id',
nextParamid: 'nextParamid',
},
},
])
Expand Down