Ensure params in query are updated for custom-routes (#9797)

* Add handling for params in destination query

* Update describe name

packages/next/next-server/server/next-server.ts

@@ -428,11 +428,23 @@ export default class Server {
             name: `${route.type} ${route.source} route`,
             fn: async (_req, res, params, _parsedUrl) => {
               const parsedDestination = parseUrl(route.destination, true)
+              const destQuery = parsedDestination.query
               let destinationCompiler = compilePathToRegex(
                 `${parsedDestination.pathname!}${parsedDestination.hash || ''}`
               )
               let newUrl
 
+              Object.keys(destQuery).forEach(key => {
+                const val = destQuery[key]
+                if (
+                  typeof val === 'string' &&
+                  val.startsWith(':') &&
+                  params[val.substr(1)]
+                ) {
+                  destQuery[key] = params[val.substr(1)]
+                }
+              })
+
               try {
                 newUrl = destinationCompiler(params)
               } catch (err) {
@@ -456,6 +468,7 @@ export default class Server {
                     ...parsedDestination,
                     pathname: parsedNewUrl.pathname,
                     hash: parsedNewUrl.hash,
+                    search: undefined,
                   })
                 )
                 res.statusCode = route.statusCode || DEFAULT_REDIRECT_STATUS
@@ -470,6 +483,7 @@ export default class Server {
               return {
                 finished: false,
                 pathname: newUrl,
+                query: parsedDestination.query,
               }
             },
           } as Route
@@ -795,6 +809,7 @@ export default class Server {
         const curUrl = parseUrl(req.url!, true)
         req.url = formatUrl({
           ...curUrl,
+          search: undefined,
           query: {
             ...curUrl.query,
             ...query,

packages/next/next-server/server/router.ts

@@ -11,6 +11,7 @@ export type RouteMatch = (pathname: string | undefined) => false | Params
 type RouteResult = {
   finished: boolean
   pathname?: string
+  query?: { [k: string]: string }
 }
 
 export type Route = {
@@ -105,6 +106,13 @@ export default class Router {
           parsedUrlUpdated.pathname = result.pathname
         }
 
+        if (result.query) {
+          parsedUrlUpdated.query = {
+            ...parsedUrlUpdated.query,
+            ...result.query,
+          }
+        }
+
         // check filesystem
         if (route.check === true) {
           for (const fsRoute of this.fsRoutes) {

test/integration/custom-routes/next.config.js

@@ -47,6 +47,10 @@ module.exports = {
           source: '/params/:something',
           destination: '/with-params',
         },
+        {
+          source: '/query-rewrite/:section/:name',
+          destination: '/with-params?first=:section&second=:name',
+        },
         {
           source: '/hidden/_next/:path*',
           destination: '/_next/:path*',
@@ -112,6 +116,10 @@ module.exports = {
           source: '/to-external',
           destination: 'https://google.com',
         },
+        {
+          source: '/query-redirect/:section/:name',
+          destination: '/with-params?first=:section&second=:name',
+        },
       ]
     },
   },

test/integration/custom-routes/test/index.test.js

@@ -4,6 +4,7 @@ import url from 'url'
 import stripAnsi from 'strip-ansi'
 import fs from 'fs-extra'
 import { join } from 'path'
+import cheerio from 'cheerio'
 import webdriver from 'next-webdriver'
 import {
   launchApp,
@@ -28,6 +29,8 @@ let stdout = ''
 let appPort
 let app
 
+const escapeRegex = str => str.replace(/[|\\{}()[\]^$+*?.-]/g, '\\$&')
+
 const runTests = (isDev = false) => {
   it('should handle one-to-one rewrite successfully', async () => {
     const html = await renderViaHTTP(appPort, '/first')
@@ -119,6 +122,33 @@ const runTests = (isDev = false) => {
     expect(html).toMatch(/hi there/)
   })
 
+  it('should allow params in query for rewrite', async () => {
+    const html = await renderViaHTTP(appPort, '/query-rewrite/hello/world?a=b')
+    const $ = cheerio.load(html)
+    expect(JSON.parse($('#__NEXT_DATA__').html()).query).toEqual({
+      first: 'hello',
+      second: 'world',
+      a: 'b',
+      section: 'hello',
+      name: 'world',
+    })
+  })
+
+  it('should allow params in query for redirect', async () => {
+    const res = await fetchViaHTTP(
+      appPort,
+      '/query-redirect/hello/world?a=b',
+      undefined,
+      {
+        redirect: 'manual',
+      }
+    )
+    const { pathname, query } = url.parse(res.headers.get('location'), true)
+    expect(res.status).toBe(307)
+    expect(pathname).toBe('/with-params')
+    expect(query).toEqual({ first: 'hello', second: 'world' })
+  })
+
   it('should overwrite param values correctly', async () => {
     const html = await renderViaHTTP(appPort, '/test-overwrite/first/second')
     expect(html).toMatch(/this-should-be-the-value/)
@@ -285,6 +315,15 @@ const runTests = (isDev = false) => {
             source: '/to-external',
             statusCode: 307,
           },
+          {
+            destination: '/with-params?first=:section&second=:name',
+            regex: normalizeRegEx(
+              '^\\/query-redirect(?:\\/([^\\/]+?))(?:\\/([^\\/]+?))$'
+            ),
+            regexKeys: ['section', 'name'],
+            source: '/query-redirect/:section/:name',
+            statusCode: 307,
+          },
         ],
         rewrites: [
           {
@@ -355,6 +394,14 @@ const runTests = (isDev = false) => {
             regex: normalizeRegEx('^\\/params(?:\\/([^\\/]+?))$'),
             regexKeys: ['something'],
           },
+          {
+            destination: '/with-params?first=:section&second=:name',
+            regex: normalizeRegEx(
+              '^\\/query-rewrite(?:\\/([^\\/]+?))(?:\\/([^\\/]+?))$'
+            ),
+            regexKeys: ['section', 'name'],
+            source: '/query-rewrite/:section/:name',
+          },
           {
             destination: '/_next/:path*',
             regex: normalizeRegEx(
@@ -388,7 +435,9 @@ const runTests = (isDev = false) => {
 
       for (const route of [...manifest.redirects, ...manifest.rewrites]) {
         expect(cleanStdout).toMatch(
-          new RegExp(`${route}.*?${route.destination}`)
+          new RegExp(
+            `${escapeRegex(route.source)}.*?${escapeRegex(route.destination)}`
+          )
         )
       }
     })
@@ -413,7 +462,7 @@ describe('Custom routes', () => {
     runTests(true)
   })
 
-  describe('production mode', () => {
+  describe('server mode', () => {
     beforeAll(async () => {
       const { stdout: buildStdout } = await nextBuild(appDir, [], {
         stdout: true,