[now-next] Implement new handles for custom routes (#3892)

This implements the new handles from https://github.com/zeit/now/pull/3876 to allow us to ensure the proper order for `rewrites`, `redirects`, and `headers` in Next.js. I also added in the tests from the Next.js [custom-routes test suite](https://github.com/zeit/next.js/tree/canary/test/integration/custom-routes) to ensure we're matching behavior. 

To help keep track of what each probe is testing I added support for parsing the `now.json` files in `testDeployment` as [JSON5](https://www.npmjs.com/package/json5) to allow adding comments before each probe. If this is undesired I can remove this specific change even though it makes managing the fixture tests much easier

package.json

@@ -23,6 +23,7 @@
     "eslint": "6.2.2",
     "eslint-config-prettier": "6.1.0",
     "husky": "3.0.4",
+    "json5": "2.1.1",
     "lint-staged": "9.2.5",
     "node-fetch": "2.6.0",
     "prettier": "1.18.2"

packages/now-next/src/index.ts

@@ -19,7 +19,7 @@ import {
   runPackageJsonScript,
   execCommand,
 } from '@now/build-utils';
-import { Route } from '@now/routing-utils';
+import { Route, Handler } from '@now/routing-utils';
 import {
   convertHeaders,
   convertRedirects,
@@ -406,6 +406,7 @@ export const build = async ({
                 // have a separate data output
                 (ssgDataRoute && ssgDataRoute.dataRoute) || dataRoute.page
               ),
+              check: true,
             });
           }
         }
@@ -500,6 +501,35 @@ export const build = async ({
         // User redirects
         ...redirects,
 
+        // Next.js pages, `static/` folder, reserved assets, and `public/`
+        // folder
+        { handle: 'filesystem' },
+
+        // These need to come before handle: miss or else they are grouped
+        // with that routing section
+        ...rewrites,
+
+        // We need to make sure to 404 for /_next after handle: miss since
+        // handle: miss is called before rewrites and to prevent rewriting
+        // /_next
+        { handle: 'miss' },
+        {
+          src: path.join(
+            '/',
+            entryDirectory,
+            '_next/static/(?:[^/]+/pages|chunks|runtime|css|media)/.+'
+          ),
+          status: 404,
+          check: true,
+          dest: '$0',
+        },
+
+        // Dynamic routes
+        // TODO: do we want to do this?: ...dynamicRoutes,
+        // (if so make sure to add any dynamic routes after handle: 'rewrite' )
+
+        // routes to call after a file has been matched
+        { handle: 'hit' },
         // Before we handle static files we need to set proper caching headers
         {
           // This ensures we only match known emitted-by-Next.js files and not
@@ -516,38 +546,16 @@ export const build = async ({
           },
           continue: true,
         },
-        {
-          src: path.join('/', entryDirectory, '_next(?!/data(?:/|$))(?:/.*)?'),
-        },
-        // Next.js pages, `static/` folder, reserved assets, and `public/`
-        // folder
-        { handle: 'filesystem' },
 
-        // This needs to come directly after handle: filesystem to make sure to
-        // 404 and clear the cache header for _next requests
-        {
-          src: path.join(
-            '/',
-            entryDirectory,
-            '_next/static/(?:[^/]+/pages|chunks|runtime|css|media)/.+'
-          ),
-          headers: {
-            'cache-control': '',
-          },
-          status: 404,
-        },
-
-        ...rewrites,
-        // Dynamic routes
-        // TODO: do we want to do this?: ...dynamicRoutes,
-
-        // 404
+        // error handling
         ...(output['404']
           ? [
+              { handle: 'error' } as Handler,
+
               {
-                src: path.join('/', entryDirectory, '.*'),
-                dest: path.join('/', entryDirectory, '404'),
                 status: 404,
+                src: path.join(entryDirectory, '.*'),
+                dest: path.join('/', entryDirectory, '404'),
               },
             ]
           : []),
@@ -1154,6 +1162,40 @@ export const build = async ({
 
       // redirects
       ...redirects,
+
+      // Next.js page lambdas, `static/` folder, reserved assets, and `public/`
+      // folder
+      { handle: 'filesystem' },
+
+      // These need to come before handle: miss or else they are grouped
+      // with that routing section
+      ...rewrites,
+
+      // We need to make sure to 404 for /_next after handle: miss since
+      // handle: miss is called before rewrites and to prevent rewriting /_next
+      { handle: 'miss' },
+      {
+        src: path.join(
+          '/',
+          entryDirectory,
+          '_next/static/(?:[^/]+/pages|chunks|runtime|css|media)/.+'
+        ),
+        status: 404,
+        check: true,
+        dest: '$0',
+      },
+
+      // routes that are called after each rewrite or after routes
+      // if there no rewrites
+      { handle: 'rewrite' },
+      // Dynamic routes
+      ...dynamicRoutes,
+
+      // /_next/data routes for getServerProps/getStaticProps pages
+      ...dataRoutes,
+
+      // routes to call after a file has been matched
+      { handle: 'hit' },
       // Before we handle static files we need to set proper caching headers
       {
         // This ensures we only match known emitted-by-Next.js files and not
@@ -1170,37 +1212,14 @@ export const build = async ({
         },
         continue: true,
       },
-      { src: path.join('/', entryDirectory, '_next(?!/data(?:/|$))(?:/.*)?') },
 
-      // Next.js page lambdas, `static/` folder, reserved assets, and `public/`
-      // folder
-      { handle: 'filesystem' },
-
-      // This needs to come directly after handle: filesystem to make sure to
-      // 404 and clear the cache header for _next requests
-      {
-        src: path.join(
-          '/',
-          entryDirectory,
-          '_next/static/(?:[^/]+/pages|chunks|runtime|css|media)/.+'
-        ),
-        headers: {
-          'cache-control': '',
-        },
-        status: 404,
-      },
-
-      ...rewrites,
-      // Dynamic routes
-      ...dynamicRoutes,
-
-      // /_next/data routes for getServerProps/getStaticProps pages
-      ...dataRoutes,
-
-      // Custom Next.js 404 page (TODO: do we want to remove this?)
+      // error handling
       ...(isLegacy
         ? []
         : [
+            // Custom Next.js 404 page
+            { handle: 'error' } as Handler,
+
             {
               src: path.join('/', entryDirectory, '.*'),
               dest: path.join(

packages/now-next/src/utils.ts

@@ -373,6 +373,7 @@ export async function getDynamicRoutes(
             return {
               src: regex,
               dest: !isDev ? path.join('/', entryDirectory, page) : page,
+              check: true,
             };
           });
       }
@@ -439,6 +440,7 @@ export async function getDynamicRoutes(
       routes.push({
         src: pageMatcher.matcher.source,
         dest,
+        check: true,
       });
     }
   });

packages/now-next/test/fixtures/23-custom-routes-verbose/.gitignore

@@ -0,0 +1 @@
+!public

packages/now-next/test/fixtures/23-custom-routes-verbose/next.config.js

@@ -0,0 +1,211 @@
+module.exports = {
+  generateBuildId() {
+    return 'testing-build-id';
+  },
+  experimental: {
+    async rewrites() {
+      return [
+        {
+          source: '/to-another',
+          destination: '/another/one',
+        },
+        {
+          source: '/nav',
+          destination: '/404',
+        },
+        {
+          source: '/hello-world',
+          destination: '/static/hello.txt',
+        },
+        {
+          source: '/',
+          destination: '/another',
+        },
+        {
+          source: '/another',
+          destination: '/multi-rewrites',
+        },
+        {
+          source: '/first',
+          destination: '/hello',
+        },
+        {
+          source: '/second',
+          destination: '/hello-again',
+        },
+        {
+          source: '/to-hello',
+          destination: '/hello',
+        },
+        {
+          source: '/blog/post-1',
+          destination: '/blog/post-2',
+        },
+        {
+          source: '/test/:path',
+          destination: '/:path',
+        },
+        {
+          source: '/test-overwrite/:something/:another',
+          destination: '/params/this-should-be-the-value',
+        },
+        {
+          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*',
+        },
+        {
+          source: '/api-hello',
+          destination: '/api/hello',
+        },
+        {
+          source: '/api-hello-regex/(.*)',
+          destination: '/api/hello?name=:1',
+        },
+        {
+          source: '/api-hello-param/:name',
+          destination: '/api/hello?hello=:name',
+        },
+        {
+          source: '/api-dynamic-param/:name',
+          destination: '/api/dynamic/:name?hello=:name',
+        },
+        {
+          source: '/:path/post-321',
+          destination: '/with-params',
+        },
+      ];
+    },
+    async redirects() {
+      return [
+        {
+          source: '/redirect/me/to-about/:lang',
+          destination: '/:lang/about',
+          permanent: false,
+        },
+        {
+          source: '/docs/router-status/:code',
+          destination: '/docs/v2/network/status-codes#:code',
+          statusCode: 301,
+        },
+        {
+          source: '/docs/github',
+          destination: '/docs/v2/advanced/now-for-github',
+          statusCode: 301,
+        },
+        {
+          source: '/docs/v2/advanced/:all(.*)',
+          destination: '/docs/v2/more/:all',
+          statusCode: 301,
+        },
+        {
+          source: '/hello/:id/another',
+          destination: '/blog/:id',
+          permanent: false,
+        },
+        {
+          source: '/redirect1',
+          destination: '/',
+          permanent: false,
+        },
+        {
+          source: '/redirect2',
+          destination: '/',
+          statusCode: 301,
+        },
+        {
+          source: '/redirect3',
+          destination: '/another',
+          statusCode: 302,
+        },
+        {
+          source: '/redirect4',
+          destination: '/',
+          permanent: true,
+        },
+        {
+          source: '/redir-chain1',
+          destination: '/redir-chain2',
+          statusCode: 301,
+        },
+        {
+          source: '/redir-chain2',
+          destination: '/redir-chain3',
+          statusCode: 302,
+        },
+        {
+          source: '/redir-chain3',
+          destination: '/',
+          statusCode: 303,
+        },
+        {
+          source: '/to-external',
+          destination: 'https://google.com',
+          permanent: false,
+        },
+        {
+          source: '/query-redirect/:section/:name',
+          destination: '/with-params?first=:section&second=:name',
+          permanent: false,
+        },
+        {
+          source: '/named-like-unnamed/:0',
+          destination: '/:0',
+          permanent: false,
+        },
+        {
+          source: '/redirect-override',
+          destination: '/thank-you-next',
+          permanent: false,
+        },
+      ];
+    },
+
+    async headers() {
+      return [
+        {
+          source: '/add-header',
+          headers: [
+            {
+              key: 'x-custom-header',
+              value: 'hello world',
+            },
+            {
+              key: 'x-another-header',
+              value: 'hello again',
+            },
+          ],
+        },
+        {
+          source: '/my-headers/(.*)',
+          headers: [
+            {
+              key: 'x-first-header',
+              value: 'first',
+            },
+            {
+              key: 'x-second-header',
+              value: 'second',
+            },
+          ],
+        },
+        {
+          source: '/:path*',
+          headers: [
+            {
+              key: 'x-something',
+              value: 'applied-everywhere',
+            },
+          ],
+        },
+      ];
+    },
+  },
+};

packages/now-next/test/fixtures/23-custom-routes-verbose/now.json

@@ -0,0 +1,243 @@
+{
+  "version": 2,
+  "builds": [{ "src": "package.json", "use": "@now/next" }],
+  "probes": [
+    // should handle one-to-one rewrite successfully
+    {
+      "path": "/first",
+      "mustContain": "hello"
+    },
+
+    // should handle chained rewrites successfully
+    {
+      "path": "/",
+      "mustContain": "multi-rewrites"
+    },
+
+    // should not match dynamic route immediately after applying header
+    {
+      "path": "/blog/post-321",
+      "mustContain": "with-params"
+    },
+    {
+      "path": "/blog/post-321",
+      "mustNotContain": "post-321"
+    },
+
+    // should handle chained redirects successfully
+    {
+      "path": "/redir-chain1",
+      "status": 301,
+      "responseHeaders": {
+        "location": "//redir-chain2/"
+      },
+      "fetchOptions": {
+        "redirect": "manual"
+      }
+    },
+    {
+      "path": "/redir-chain2",
+      "status": 302,
+      "responseHeaders": {
+        "location": "//redir-chain3/"
+      },
+      "fetchOptions": {
+        "redirect": "manual"
+      }
+    },
+    {
+      "path": "/redir-chain3",
+      "status": 303,
+      "responseHeaders": {
+        "location": "//$/"
+      },
+      "fetchOptions": {
+        "redirect": "manual"
+      }
+    },
+
+    // should redirect successfully with permanent: false
+    {
+      "path": "/redirect1",
+      "status": 307,
+      "responseHeaders": {
+        "location": "//$/"
+      },
+      "fetchOptions": {
+        "redirect": "manual"
+      }
+    },
+
+    // should redirect with params successfully
+    {
+      "path": "/hello/123/another",
+      "status": 307,
+      "responseHeaders": {
+        "location": "//blog/123/"
+      },
+      "fetchOptions": {
+        "redirect": "manual"
+      }
+    },
+
+    // should redirect with hash successfully
+    {
+      "path": "/docs/router-status/500",
+      "status": 301,
+      "responseHeaders": {
+        "location": "/#500$/"
+      },
+      "fetchOptions": {
+        "redirect": "manual"
+      }
+    },
+
+    // should redirect successfully with provided statusCode
+    {
+      "path": "/redirect2",
+      "status": 301,
+      "responseHeaders": {
+        "location": "//$/"
+      },
+      "fetchOptions": {
+        "redirect": "manual"
+      }
+    },
+
+    // should server static files through a rewrite
+    {
+      "path": "/hello-world",
+      "mustContain": "hello world!"
+    },
+
+    // should rewrite with params successfully
+    {
+      "path": "/test/hello",
+      "mustContain": "Hello"
+    },
+
+    // should double redirect successfully
+    {
+      "path": "/docs/github",
+      "mustContain": "hi there"
+    },
+
+    // should allow params in query for rewrite
+    {
+      "path": "/query-rewrite/hello/world?a=b",
+      "mustContain": "\"a\":\"b\""
+    },
+    {
+      "path": "/query-rewrite/hello/world?a=b",
+      "mustContain": "\"section\":\"hello\""
+    },
+    {
+      "path": "/query-rewrite/hello/world?a=b",
+      "mustContain": "\"name\":\"world\""
+    },
+    {
+      "path": "/query-rewrite/hello/world?a=b",
+      "mustContain": "\"first\":\"hello\""
+    },
+    {
+      "path": "/query-rewrite/hello/world?a=b",
+      "mustContain": "\"second\":\"world\""
+    },
+
+    // should not allow rewrite to override page file
+    {
+      "path": "/nav",
+      "mustContain": "to-hello"
+    },
+
+    // show allow redirect to override the page
+    {
+      "path": "/redirect-override",
+      "status": 307,
+      "responseHeaders": {
+        "location": "//thank-you-next$/"
+      },
+      "fetchOptions": {
+        "redirect": "manual"
+      }
+    },
+
+    // should match a page after a rewrite
+    {
+      "path": "/to-hello",
+      "mustContain": "Hello"
+    },
+
+    // should match dynamic route after rewrite
+    {
+      "path": "/blog/post-1",
+      "mustContain": "post-2"
+    },
+
+    // should match public file after rewrite
+    {
+      "path": "/blog/data.json",
+      "mustContain": "\"hello\": \"world\""
+    },
+
+    // should match /_next file after rewrite
+    {
+      "path": "/hidden/_next/static/testing-build-id/pages/hello.js",
+      "mustContain": "createElement"
+    },
+
+    // should allow redirecting to external resource
+    {
+      "path": "/to-external",
+      "status": 307,
+      "responseHeaders": {
+        "location": "/google.com/"
+      },
+      "fetchOptions": {
+        "redirect": "manual"
+      }
+    },
+
+    // should apply headers for exact match
+    {
+      "path": "/add-header",
+      "responseHeaders": {
+        "x-custom-header": "hello world",
+        "x-another-header": "hello again"
+      }
+    },
+
+    // should apply headers for multi match
+    {
+      "path": "/my-headers/first",
+      "responseHeaders": {
+        "x-first-header": "first",
+        "x-second-header": "second"
+      }
+    },
+
+    // should handle basic api rewrite successfully
+    {
+      "path": "/api-hello",
+      "mustContain": "{\"query\":{}}"
+    },
+
+    // should handle api rewrite with param successfully
+    {
+      "path": "/api-hello-param/hello",
+      "mustContain": "{\"query\":{\"hello\":\"hello\",\"name\":\"hello\"}}"
+    },
+
+    // should handle encoded value in the pathname correctly
+    {
+      "path": "/redirect/me/to-about/%5Cgoogle.com",
+      "status": 307,
+      "responseHeaders": {
+        "location": "/%5Cgoogle.com/about/"
+      },
+      "fetchOptions": {
+        "redirect": "manual"
+      }
+    }
+  ]
+}

packages/now-next/test/fixtures/23-custom-routes-verbose/package.json

@@ -0,0 +1,7 @@
+{
+  "dependencies": {
+    "next": "canary",
+    "react": "^16.8.6",
+    "react-dom": "^16.8.6"
+  }
+}

packages/now-next/test/fixtures/23-custom-routes-verbose/pages/another/[id].js

@@ -0,0 +1 @@
+export default () => 'hi'

packages/now-next/test/fixtures/23-custom-routes-verbose/pages/api/dynamic/[slug].js

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

packages/now-next/test/fixtures/23-custom-routes-verbose/pages/api/hello.js

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

packages/now-next/test/fixtures/23-custom-routes-verbose/pages/blog/[post]/index.js

@@ -0,0 +1,11 @@
+import { useRouter } from 'next/router'
+
+const Page = () => (
+  <>
+    <p>post: {useRouter().query.post}</p>
+  </>
+)
+
+Page.getInitialProps = () => ({ hello: 'world' })
+
+export default Page

packages/now-next/test/fixtures/23-custom-routes-verbose/pages/docs/v2/more/now-for-github.js

@@ -0,0 +1 @@
+export default () => 'hi there';

packages/now-next/test/fixtures/23-custom-routes-verbose/pages/hello-again.js

@@ -0,0 +1,10 @@
+import Link from 'next/link';
+
+export default () => (
+  <>
+    <h3 id="hello-again">Hello again</h3>
+    <Link href="/nav">
+      <a id="to-nav">to nav</a>
+    </Link>
+  </>
+);

packages/now-next/test/fixtures/23-custom-routes-verbose/pages/hello.js

@@ -0,0 +1,14 @@
+import Link from 'next/link';
+
+const Page = () => (
+  <>
+    <h3 id="hello">Hello</h3>
+    <Link href="/nav">
+      <a id="to-nav">to nav</a>
+    </Link>
+  </>
+);
+
+Page.getInitialProps = () => ({ hello: 'world' });
+
+export default Page;

packages/now-next/test/fixtures/23-custom-routes-verbose/pages/multi-rewrites.js

@@ -0,0 +1 @@
+export default () => 'multi-rewrites';

packages/now-next/test/fixtures/23-custom-routes-verbose/pages/nav.js

@@ -0,0 +1,13 @@
+import Link from 'next/link';
+
+export default () => (
+  <>
+    <h3 id="nav">Nav</h3>
+    <Link href="/hello" as="/first">
+      <a id="to-hello">to hello</a>
+    </Link>
+    <Link href="/hello-again" as="/second">
+      <a id="to-hello-again">to hello-again</a>
+    </Link>
+  </>
+);

packages/now-next/test/fixtures/23-custom-routes-verbose/pages/params.js

@@ -0,0 +1,10 @@
+import { useRouter } from 'next/router';
+
+const Page = () => {
+  const { query } = useRouter();
+  return <p>{JSON.stringify(query)}</p>;
+};
+
+Page.getInitialProps = () => ({ a: 'b' });
+
+export default Page;

packages/now-next/test/fixtures/23-custom-routes-verbose/pages/redirect-override.js

@@ -0,0 +1 @@
+export default () => 'got to the page';

packages/now-next/test/fixtures/23-custom-routes-verbose/pages/with-params.js

@@ -0,0 +1,10 @@
+import { useRouter } from 'next/router';
+
+const Page = () => {
+  const { query } = useRouter();
+  return <p>{JSON.stringify(query)}</p>;
+};
+
+Page.getInitialProps = () => ({ hello: 'GIPGIP' });
+
+export default Page;

packages/now-next/test/fixtures/23-custom-routes-verbose/public/blog/data.json

@@ -0,0 +1,3 @@
+{
+  "hello": "world"
+}

packages/now-next/test/fixtures/23-custom-routes-verbose/public/static/hello.txt

@@ -0,0 +1 @@
+hello world!

packages/now-next/test/unit/build.test.js

@@ -117,6 +117,7 @@ describe('build meta dev', () => {
       {
         src: '^/(nested\\/([^/]+?)(?:\\/)?)$',
         dest: 'http://localhost:5000/$1',
+        check: true,
       },
       { src: '/data.txt', dest: 'http://localhost:5000/data.txt' },
     ]);

packages/now-routing-utils/src/types.ts

@@ -25,6 +25,9 @@ export type Source = {
 
 export type Handler = {
   handle: HandleValue;
+  src?: string;
+  dest?: string;
+  status?: number;
 };
 
 export type Route = Source | Handler;

test/lib/deployment/test-deployment.js

@@ -1,6 +1,7 @@
 const assert = require('assert');
 const bufferReplace = require('buffer-replace');
 const fs = require('fs');
+const json5 = require('json5');
 const glob = require('util').promisify(require('glob'));
 const path = require('path');
 const { spawn } = require('child_process');
@@ -52,7 +53,8 @@ async function testDeployment(
     );
   }
 
-  const nowJson = JSON.parse(bodies['now.json']);
+  // we use json5 to allow comments for probes
+  const nowJson = json5.parse(bodies['now.json']);
 
   if (process.env.NOW_BUILDER_DEBUG) {
     if (!nowJson.build) {

yarn.lock

@@ -6900,6 +6900,13 @@ json-stringify-safe@^5.0.1, json-stringify-safe@~5.0.1:
   resolved "https://registry.yarnpkg.com/json-stringify-safe/-/json-stringify-safe-5.0.1.tgz#1296a2d58fd45f19a0f6ce01d65701e2c735b6eb"
   integrity sha1-Epai1Y/UXxmg9s4B1lcB4sc1tus=
 
+json5@2.1.1:
+  version "2.1.1"
+  resolved "https://registry.yarnpkg.com/json5/-/json5-2.1.1.tgz#81b6cb04e9ba496f1c7005d07b4368a2638f90b6"
+  integrity sha512-l+3HXD0GEI3huGq1njuqtzYK8OYJyXMkOLtQ53pjWh89tvWS2h6l+1zMkYWqlb57+SiQodKZyvMEFb2X+KrFhQ==
+  dependencies:
+    minimist "^1.2.0"
+
 json5@2.x, json5@^2.1.0:
   version "2.1.0"
   resolved "https://registry.yarnpkg.com/json5/-/json5-2.1.0.tgz#e7a0c62c48285c628d20a10b85c89bb807c32850"