Make sure rewrites are handled in serverless mode correctly (#10697)

Co-authored-by: Joe Haddad <timer150@gmail.com>

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

@@ -93,26 +93,22 @@ const nextServerlessLoader: loader.Loader = function() {
 
   const handleRewrites = `
     const getCustomRouteMatcher = pathMatch(true)
+    const {prepareDestination} = require('next/dist/next-server/server/router')
 
     function handleRewrites(parsedUrl) {
       for (const rewrite of rewrites) {
         const matcher = getCustomRouteMatcher(rewrite.source)
         const params = matcher(parsedUrl.pathname)
 
         if (params) {
-          parsedUrl.query = {
-            ...parsedUrl.query,
-            ...params
-          }
-          const parsedDest = parse(rewrite.destination)
-          const destCompiler = pathToRegexp.compile(
-            \`\${parsedDest.pathname}\${parsedDest.hash || ''}\`
+          const { parsedDestination } = prepareDestination(
+            rewrite.destination,
+            params
           )
-          const newUrl = destCompiler(params)
-          const parsedNewUrl = parse(newUrl)
+          Object.assign(parsedUrl.query, parsedDestination.query, params)
+          delete parsedDestination.query
 
-          parsedUrl.pathname = parsedNewUrl.pathname
-          parsedUrl.hash = parsedNewUrl.hash
+          Object.assign(parsedUrl, parsedDestination)
 
           if (parsedUrl.pathname === '${page}'){
             break
@@ -170,7 +166,7 @@ const nextServerlessLoader: loader.Loader = function() {
           `
               : ''
           }
-          const parsedUrl = parse(req.url, true)
+          const parsedUrl = handleRewrites(parse(req.url, true))
 
           const params = ${
             pageIsDynamicRoute

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

@@ -73,7 +73,11 @@ module.exports = {
         },
         {
           source: '/api-hello-param/:name',
-          destination: '/api/hello?name=:name',
+          destination: '/api/hello?hello=:name',
+        },
+        {
+          source: '/api-dynamic-param/:name',
+          destination: '/api/dynamic/:name?hello=:name',
         },
         {
           source: '/:path/post-321',

test/integration/custom-routes/pages/api/dynamic/[slug].js

@@ -0,0 +1 @@
+export default async (req, res) => res.json({ query: req.query })

test/integration/custom-routes/server.js

@@ -0,0 +1,36 @@
+const path = require('path')
+const http = require('http')
+
+const server = http.createServer((req, res) => {
+  const pagePath = page => path.join('.next/serverless/pages/', page)
+  const render = page => {
+    require(`./${pagePath(page)}`).render(req, res)
+  }
+  const apiCall = page => {
+    require(`./${pagePath(page)}`).default(req, res)
+  }
+
+  switch (req.url) {
+    case '/blog/post-1': {
+      return render('/blog/[post]')
+    }
+    case '/query-rewrite/first/second': {
+      return render('/with-params')
+    }
+    case '/api-hello-param/first': {
+      return apiCall('/api/hello')
+    }
+    case '/api-dynamic-param/first': {
+      return apiCall('/api/dynamic/[slug]')
+    }
+    default: {
+      res.statusCode(404)
+      return res.end('404')
+    }
+  }
+})
+
+const port = process.env.PORT || 3000
+server.listen(port, () => {
+  console.log('ready on', port)
+})

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

@@ -18,6 +18,7 @@ import {
   getBrowserBodyText,
   waitFor,
   normalizeRegEx,
+  initNextServerScript,
 } from 'next-test-utils'
 
 jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2
@@ -308,7 +309,9 @@ const runTests = (isDev = false) => {
 
   it('should handle api rewrite with param successfully', async () => {
     const data = await renderViaHTTP(appPort, '/api-hello-param/hello')
-    expect(JSON.parse(data)).toEqual({ query: { name: 'hello' } })
+    expect(JSON.parse(data)).toEqual({
+      query: { name: 'hello', hello: 'hello' },
+    })
   })
 
   it('should handle encoded value in the pathname correctly', async () => {
@@ -593,10 +596,15 @@ const runTests = (isDev = false) => {
             source: '/api-hello-regex/(.*)',
           },
           {
-            destination: '/api/hello?name=:name',
+            destination: '/api/hello?hello=:name',
             regex: normalizeRegEx('^\\/api-hello-param(?:\\/([^\\/]+?))$'),
             source: '/api-hello-param/:name',
           },
+          {
+            destination: '/api/dynamic/:name?hello=:name',
+            regex: normalizeRegEx('^\\/api-dynamic-param(?:\\/([^\\/]+?))$'),
+            source: '/api-dynamic-param/:name',
+          },
           {
             destination: '/with-params',
             regex: normalizeRegEx('^(?:\\/([^\\/]+?))\\/post-321$'),
@@ -608,6 +616,10 @@ const runTests = (isDev = false) => {
             page: '/another/[id]',
             regex: normalizeRegEx('^\\/another\\/([^\\/]+?)(?:\\/)?$'),
           },
+          {
+            page: '/api/dynamic/[slug]',
+            regex: normalizeRegEx('^\\/api\\/dynamic\\/([^\\/]+?)(?:\\/)?$'),
+          },
           {
             page: '/blog/[post]',
             regex: normalizeRegEx('^\\/blog\\/([^\\/]+?)(?:\\/)?$'),
@@ -723,10 +735,77 @@ describe('Custom routes', () => {
       buildId = await fs.readFile(join(appDir, '.next/BUILD_ID'), 'utf8')
     })
     afterAll(async () => {
-      await killApp(app)
       await fs.writeFile(nextConfigPath, nextConfigContent, 'utf8')
+      await killApp(app)
     })
 
     runTests()
   })
+
+  describe('raw serverless mode', () => {
+    beforeAll(async () => {
+      nextConfigContent = await fs.readFile(nextConfigPath, 'utf8')
+      await fs.writeFile(
+        nextConfigPath,
+        nextConfigContent.replace(/\/\/ target/, 'target'),
+        'utf8'
+      )
+      await nextBuild(appDir)
+
+      appPort = await findPort()
+      app = await initNextServerScript(join(appDir, 'server.js'), /ready on/, {
+        ...process.env,
+        PORT: appPort,
+      })
+    })
+    afterAll(async () => {
+      await fs.writeFile(nextConfigPath, nextConfigContent, 'utf8')
+      await killApp(app)
+    })
+
+    it('should apply rewrites in lambda correctly for page route', async () => {
+      const html = await renderViaHTTP(appPort, '/query-rewrite/first/second')
+      const data = JSON.parse(
+        cheerio
+          .load(html)('p')
+          .text()
+      )
+      expect(data).toEqual({
+        first: 'first',
+        second: 'second',
+        section: 'first',
+        name: 'second',
+      })
+    })
+
+    it('should apply rewrites in lambda correctly for dynamic route', async () => {
+      const html = await renderViaHTTP(appPort, '/blog/post-1')
+      expect(html).toContain('post-2')
+    })
+
+    it('should apply rewrites in lambda correctly for API route', async () => {
+      const data = JSON.parse(
+        await renderViaHTTP(appPort, '/api-hello-param/first')
+      )
+      expect(data).toEqual({
+        query: {
+          name: 'first',
+          hello: 'first',
+        },
+      })
+    })
+
+    it('should apply rewrites in lambda correctly for dynamic API route', async () => {
+      const data = JSON.parse(
+        await renderViaHTTP(appPort, '/api-dynamic-param/first')
+      )
+      expect(data).toEqual({
+        query: {
+          slug: 'first',
+          name: 'first',
+          hello: 'first',
+        },
+      })
+    })
+  })
 })