fix(core): support no-op rewrites (#1329)

serverless-nextjs · Jun 30, 2021 · 27d1943 · 27d1943
1 parent df2b703
commit 27d1943
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 6 deletions.
diff --git a/packages/e2e-tests/next-app/cypress/integration/rewrites.test.ts b/packages/e2e-tests/next-app/cypress/integration/rewrites.test.ts
@@ -70,6 +70,11 @@ describe("Rewrites Tests", () => {
         path: "/rewrite-dest-with-query?a=b",
         expectedRewrite: "/ssr-page?a=b&foo=bar",
         expectedStatus: 200
+      },
+      {
+        path: "/no-op-rewrite",
+        expectedRewrite: "/ssr-page",
+        expectedStatus: 200
       }
     ].forEach(({ path, expectedRewrite, expectedStatus }) => {
       it(`rewrites path ${path} to ${expectedRewrite}`, () => {

diff --git a/packages/e2e-tests/next-app/next.config.js b/packages/e2e-tests/next-app/next.config.js
@@ -146,6 +146,14 @@ module.exports = {
         source: "/api/external-rewrite-issues-with-query",
         destination:
           "https://api.github.com/repos/serverless-nextjs/serverless-next.js/issues?a=b"
+      },
+      {
+        source: "/no-op-rewrite",
+        destination: "/no-op-rewrite"
+      },
+      {
+        source: "/no-op-rewrite",
+        destination: "/ssr-page"
       }
     ];
   },

diff --git a/packages/libs/core/src/route/page.ts b/packages/libs/core/src/route/page.ts
@@ -64,7 +64,7 @@ export const handlePageReq = (
     };
   }
 
-  const rewrite = !isRewrite && getRewritePath(uri, routesManifest);
+  const rewrite = !isRewrite && getRewritePath(uri, routesManifest, manifest);
   if (rewrite) {
     const [path, querystring] = rewrite.split("?");
     if (isExternalRewrite(path)) {

diff --git a/packages/libs/core/src/route/rewrite.ts b/packages/libs/core/src/route/rewrite.ts
@@ -1,15 +1,18 @@
 import { addDefaultLocaleToPath } from "./locale";
 import { compileDestination, matchPath } from "../match";
-import { RewriteData, RoutesManifest } from "../types";
+import { PageManifest, RewriteData, RoutesManifest } from "../types";
+import { handlePageReq } from "../route/page";
 
 /**
  * Get the rewrite of the given path, if it exists.
- * @param path
+ * @param uri
+ * @param pageManifest
  * @param routesManifest
  */
 export function getRewritePath(
   uri: string,
-  routesManifest: RoutesManifest
+  routesManifest: RoutesManifest,
+  pageManifest?: PageManifest
 ): string | undefined {
   const path = addDefaultLocaleToPath(uri, routesManifest);
 
@@ -27,6 +30,21 @@ export function getRewritePath(
       return;
     }
 
+    // No-op rewrite support for pages: skip to next rewrite if path does not map to existing non-dynamic and dynamic routes
+    if (pageManifest && path === destination) {
+      const url = handlePageReq(
+        destination,
+        pageManifest,
+        routesManifest,
+        false,
+        true
+      );
+
+      if (url.statusCode === 404) {
+        continue;
+      }
+    }
+
     // Pass unused params to destination
     // except nextInternalLocale param since it's already in path prefix
     const querystring = Object.keys(params)

diff --git a/packages/libs/core/tests/route/rewrite.test.ts b/packages/libs/core/tests/route/rewrite.test.ts
@@ -1,9 +1,10 @@
 import { getRewritePath, isExternalRewrite } from "../../src/route/rewrite";
-import { RoutesManifest } from "../../src/types";
+import { PageManifest, RoutesManifest } from "../../src/types";
 
 describe("Rewriter Tests", () => {
   describe("getRewritePath()", () => {
     let routesManifest: RoutesManifest;
+    let pageManifest: PageManifest;
 
     beforeAll(() => {
       routesManifest = {
@@ -47,10 +48,39 @@ describe("Rewriter Tests", () => {
           {
             source: "/multi-query/:path*",
             destination: "/target"
+          },
+          {
+            source: "/no-op-rewrite",
+            destination: "/no-op-rewrite"
+          },
+          {
+            source: "/no-op-rewrite",
+            destination: "/actual-no-op-rewrite-destination"
           }
         ],
         redirects: []
       };
+
+      pageManifest = {
+        publicFiles: {},
+        trailingSlash: false,
+        buildId: "test-build-id",
+        pages: {
+          dynamic: [],
+          html: {
+            dynamic: {},
+            nonDynamic: {}
+          },
+          ssg: {
+            dynamic: {},
+            nonDynamic: {}
+          },
+          ssr: {
+            dynamic: {},
+            nonDynamic: {}
+          }
+        }
+      };
     });
 
     it.each`
@@ -68,10 +98,11 @@ describe("Rewriter Tests", () => {
       ${"/query/foo"}           | ${"/target?a=b&path=foo"}
       ${"/manual-query/foo"}    | ${"/target?key=foo"}
       ${"/multi-query/foo/bar"} | ${"/target?path=foo&path=bar"}
+      ${"/no-op-rewrite"}       | ${"/actual-no-op-rewrite-destination"}
     `(
       "rewrites path $path to $expectedRewrite",
       ({ path, expectedRewrite }) => {
-        const rewrite = getRewritePath(path, routesManifest);
+        const rewrite = getRewritePath(path, routesManifest, pageManifest);
 
         if (expectedRewrite) {
           expect(rewrite).toEqual(expectedRewrite);