Make sure to pass through query values for custom routes

packages/next/build/webpack/loaders/next-serverless-loader.ts

@@ -103,7 +103,8 @@ const nextServerlessLoader: loader.Loader = function() {
         if (params) {
           const { parsedDestination } = prepareDestination(
             rewrite.destination,
-            params
+            params,
+            parsedUrl.query
           )
           Object.assign(parsedUrl.query, parsedDestination.query, params)
           delete parsedDestination.query

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

@@ -496,10 +496,11 @@ export default class Server {
           match: route.match,
           statusCode: route.statusCode,
           name: `Redirect route`,
-          fn: async (_req, res, params, _parsedUrl) => {
+          fn: async (_req, res, params, parsedUrl) => {
             const { parsedDestination } = prepareDestination(
               route.destination,
               params,
+              parsedUrl.query,
               true
             )
             const updatedDestination = formatUrl(parsedDestination)
@@ -528,10 +529,11 @@ export default class Server {
           type: route.type,
           name: `Rewrite route`,
           match: route.match,
-          fn: async (req, res, params, _parsedUrl) => {
+          fn: async (req, res, params, parsedUrl) => {
             const { newUrl, parsedDestination } = prepareDestination(
               route.destination,
-              params
+              params,
+              parsedUrl.query
             )
 
             // external rewrite, proxy it

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

@@ -1,5 +1,6 @@
 import { IncomingMessage, ServerResponse } from 'http'
 import { parse as parseUrl, UrlWithParsedQuery } from 'url'
+import { ParsedUrlQuery } from 'querystring'
 import { compile as compilePathToRegex } from 'next/dist/compiled/path-to-regexp'
 import pathMatch from './lib/path-match'
 
@@ -36,6 +37,7 @@ export type PageChecker = (pathname: string) => Promise<boolean>
 export const prepareDestination = (
   destination: string,
   params: Params,
+  query: ParsedUrlQuery,
   isRedirect?: boolean
 ) => {
   const parsedDestination = parseUrl(destination, true)
@@ -62,7 +64,8 @@ export const prepareDestination = (
     destQuery[key] = value
   }
 
-  // add params to query if it's not a redirect
+  // add path params to query if it's not a redirect and not
+  // already defined in destination query
   if (!isRedirect) {
     for (const [name, value] of Object.entries(params)) {
       if (!(name in destQuery)) {
@@ -87,6 +90,16 @@ export const prepareDestination = (
     }
     throw err
   }
+
+  // Query merge order lowest priority to highest
+  // 1. initial URL query values
+  // 2. path segment values
+  // 3. destination specified query values
+  parsedDestination.query = {
+    ...query,
+    ...parsedDestination.query,
+  }
+
   return {
     newUrl,
     parsedDestination,
@@ -203,11 +216,6 @@ export default class Router {
 
       // Check if the match function matched
       if (newParams) {
-        // Combine parameters and querystring
-        if (route.type === 'rewrite' || route.type === 'redirect') {
-          parsedUrlUpdated.query = { ...parsedUrlUpdated.query, ...newParams }
-        }
-
         const result = await route.fn(req, res, newParams, parsedUrlUpdated)
 
         // The response was handled

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

@@ -181,6 +181,7 @@ const runTests = (isDev = false) => {
     expect(query).toEqual({
       first: 'hello',
       second: 'world',
+      a: 'b',
     })
   })
 
@@ -191,6 +192,31 @@ const runTests = (isDev = false) => {
     expect(html).toMatch(/second/)
   })
 
+  it('should handle query for rewrite correctly', async () => {
+    // query merge order lowest priority to highest
+    // 1. initial URL query values
+    // 2. path segment values
+    // 3. destination specified query values
+
+    const html = await renderViaHTTP(
+      appPort,
+      '/query-rewrite/first/second?section=overridden&name=overridden&first=overridden&second=overridden&keep=me'
+    )
+
+    const data = JSON.parse(
+      cheerio
+        .load(html)('p')
+        .text()
+    )
+    expect(data).toEqual({
+      first: 'first',
+      second: 'second',
+      section: 'first',
+      name: 'second',
+      keep: 'me',
+    })
+  })
+
   // current routes order do not allow rewrites to override page
   // but allow redirects to
   it('should not allow rewrite to override page file', async () => {
@@ -281,9 +307,26 @@ const runTests = (isDev = false) => {
   })
 
   it('should support proxying to external resource', async () => {
-    const res = await fetchViaHTTP(appPort, '/proxy-me/first')
+    const res = await fetchViaHTTP(appPort, '/proxy-me/first?keep=me&and=me')
     expect(res.status).toBe(200)
-    expect([...externalServerHits]).toEqual(['/first?path=first'])
+    expect(
+      [...externalServerHits].map(u => {
+        const { pathname, query } = url.parse(u, true)
+        return {
+          pathname,
+          query,
+        }
+      })
+    ).toEqual([
+      {
+        pathname: '/first',
+        query: {
+          path: 'first',
+          keep: 'me',
+          and: 'me',
+        },
+      },
+    ])
     expect(await res.text()).toContain('hi from external')
   })