From b818b5481d79beb5b73c9b7f9e3e6eb87e86cc9c Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 13 Aug 2020 23:21:46 -0500 Subject: [PATCH 1/2] Update rewrite params query updating --- .../webpack/loaders/next-serverless-loader.ts | 2 +- .../lib/router/utils/prepare-destination.ts | 30 +++++++++++++------ .../custom-routes/test/index.test.js | 8 +++-- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index 7ea31a91aa6ca..5adf4f07dad03 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -163,7 +163,7 @@ const nextServerlessLoader: loader.Loader = function () { "${basePath}" ) - Object.assign(parsedUrl.query, parsedDestination.query, params) + Object.assign(parsedUrl.query, parsedDestination.query) delete parsedDestination.query Object.assign(parsedUrl, parsedDestination) diff --git a/packages/next/next-server/lib/router/utils/prepare-destination.ts b/packages/next/next-server/lib/router/utils/prepare-destination.ts index f4cc6bf06b0d9..1e297a7fca89e 100644 --- a/packages/next/next-server/lib/router/utils/prepare-destination.ts +++ b/packages/next/next-server/lib/router/utils/prepare-destination.ts @@ -1,7 +1,7 @@ import { ParsedUrlQuery } from 'querystring' import { searchParamsToUrlQuery } from './querystring' import { parseRelativeUrl } from './parse-relative-url' -import { compile as compilePathToRegex } from 'next/dist/compiled/path-to-regexp' +import * as pathToRegexp from 'next/dist/compiled/path-to-regexp' type Params = { [param: string]: any } @@ -49,9 +49,16 @@ export default function prepareDestination( parsedDestination.searchParams ) const destQuery = parsedDestination.query + const destPath = `${parsedDestination.pathname!}${ + parsedDestination.hash || '' + }` + const destPathParamKeys: pathToRegexp.Key[] = [] + pathToRegexp.pathToRegexp(destPath, destPathParamKeys) - let destinationCompiler = compilePathToRegex( - `${parsedDestination.pathname!}${parsedDestination.hash || ''}`, + const destPathParams = destPathParamKeys.map((key) => key.name) + + let destinationCompiler = pathToRegexp.compile( + destPath, // we don't validate while compiling the destination since we should // have already validated before we got to this point and validating // breaks compiling destinations with named pattern params from the source @@ -69,18 +76,23 @@ export default function prepareDestination( // the value needs to start with a forward-slash to be compiled // correctly value = `/${value}` - const queryCompiler = compilePathToRegex(value, { validate: false }) + const queryCompiler = pathToRegexp.compile(value, { validate: false }) value = queryCompiler(params).substr(1) } destQuery[key] = value } // add path params to query if it's not a redirect and not - // already defined in destination query - if (appendParamsToQuery) { - for (const [name, value] of Object.entries(params)) { - if (!(name in destQuery)) { - destQuery[name] = value + // already defined in destination query or path + const paramKeys = Object.keys(params) + + if ( + appendParamsToQuery && + !paramKeys.some((key) => destPathParams.includes(key)) + ) { + for (const key of paramKeys) { + if (!(key in destQuery)) { + destQuery[key] = params[key] } } } diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js index 2128a9abd4a19..d3acb92fa4d41 100644 --- a/test/integration/custom-routes/test/index.test.js +++ b/test/integration/custom-routes/test/index.test.js @@ -173,6 +173,12 @@ const runTests = (isDev = false) => { expect(html).toMatch(/Hello/) }) + it('should not append params when one is used in destination path', async () => { + const html = await renderViaHTTP(appPort, '/test/with-params?a=b') + const $ = cheerio.load(html) + expect(JSON.parse($('p').text())).toEqual({ a: 'b' }) + }) + it('should double redirect successfully', async () => { const html = await renderViaHTTP(appPort, '/docs/github') expect(html).toMatch(/hi there/) @@ -397,7 +403,6 @@ const runTests = (isDev = false) => { { pathname: '/first', query: { - path: 'first', keep: 'me', and: 'me', }, @@ -1164,7 +1169,6 @@ describe('Custom routes', () => { expect(data).toEqual({ query: { slug: 'first', - name: 'first', hello: 'first', }, }) From 021febfbde34c9ae3e0462bd2dbeb5c141795178 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Fri, 14 Aug 2020 10:26:58 -0500 Subject: [PATCH 2/2] Update prerender test --- test/integration/prerender/test/index.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/prerender/test/index.test.js b/test/integration/prerender/test/index.test.js index 4a79ded6d4a27..f8e6b6810d30f 100644 --- a/test/integration/prerender/test/index.test.js +++ b/test/integration/prerender/test/index.test.js @@ -810,7 +810,6 @@ const runTests = (dev = false, isEmulatedServerless = false) => { post: `post-${item}`, }) expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({ - item: item + '', post: `post-${item}`, }) })