From a7af013350e95a44cf7d991d1187ec0b40cb642b Mon Sep 17 00:00:00 2001
From: Jan Potoms <2109932+Janpot@users.noreply.github.com>
Date: Mon, 22 Jun 2020 20:16:21 +0200
Subject: [PATCH] Update route regex for optional catch-all parameters in named
 regexes (#14456)

Noticed while working on https://github.com/vercel/next.js/pull/14400 that the optional catch-all handling was missing in `namedRegex`.

This whole file also seemed quite regex heavy so I took a look at the overall logic and changed a few things. It worked by regex escaping the whole route then unescape the dynamic parts. I changed it to only regex escape the static parts, this eliminates unnecessary back and forth escaping. It also makes the dynamic parts handling more readable. The whole logic is less reliant on regexes and just uses simple string manipulation to translate the route into a regex, I didn't measure anything but as an effect this should make it more performant.
---
 .../lib/router/utils/route-regex.ts           | 86 ++++++++++---------
 test/integration/prerender/test/index.test.js |  2 +-
 2 files changed, 48 insertions(+), 40 deletions(-)

diff --git a/packages/next/next-server/lib/router/utils/route-regex.ts b/packages/next/next-server/lib/router/utils/route-regex.ts
index 5d26a24bb3ee9..7d75fbcc83a70 100644
--- a/packages/next/next-server/lib/router/utils/route-regex.ts
+++ b/packages/next/next-server/lib/router/utils/route-regex.ts
@@ -1,3 +1,9 @@
+interface Group {
+  pos: number
+  repeat: boolean
+  optional: boolean
+}
+
 // this isn't importing the escape-string-regex module
 // to reduce bytes
 function escapeRegex(str: string) {
@@ -5,17 +11,15 @@ function escapeRegex(str: string) {
 }
 
 function parseParameter(param: string) {
-  const optional = /^\\\[.*\\\]$/.test(param)
+  const optional = param.startsWith('[') && param.endsWith(']')
   if (optional) {
-    param = param.slice(2, -2)
+    param = param.slice(1, -1)
   }
-  const repeat = /^(\\\.){3}/.test(param)
+  const repeat = param.startsWith('...')
   if (repeat) {
-    param = param.slice(6)
+    param = param.slice(3)
   }
-  // Un-escape key
-  const key = param.replace(/\\([|\\{}()[\]^$+*?.-])/g, '$1')
-  return { key, repeat, optional }
+  return { key: param, repeat, optional }
 }
 
 export function getRouteRegex(
@@ -24,48 +28,52 @@ export function getRouteRegex(
   re: RegExp
   namedRegex?: string
   routeKeys?: { [named: string]: string }
-  groups: {
-    [groupName: string]: { pos: number; repeat: boolean; optional: boolean }
-  }
+  groups: { [groupName: string]: Group }
 } {
-  // Escape all characters that could be considered RegEx
-  const escapedRoute = escapeRegex(normalizedRoute.replace(/\/$/, '') || '/')
+  const segments = (normalizedRoute.replace(/\/$/, '') || '/')
+    .slice(1)
+    .split('/')
 
-  const groups: {
-    [groupName: string]: { pos: number; repeat: boolean; optional: boolean }
-  } = {}
+  const groups: { [groupName: string]: Group } = {}
   let groupIndex = 1
-
-  const parameterizedRoute = escapedRoute.replace(
-    /\/\\\[([^/]+?)\\\](?=\/|$)/g,
-    (_, $1) => {
-      const { key, optional, repeat } = parseParameter($1)
-      groups[key] = { pos: groupIndex++, repeat, optional }
-      return repeat ? (optional ? '(?:/(.+?))?' : '/(.+?)') : '/([^/]+?)'
-    }
-  )
-
-  let namedParameterizedRoute: string | undefined
+  const parameterizedRoute = segments
+    .map((segment) => {
+      if (segment.startsWith('[') && segment.endsWith(']')) {
+        const { key, optional, repeat } = parseParameter(segment.slice(1, -1))
+        groups[key] = { pos: groupIndex++, repeat, optional }
+        return repeat ? (optional ? '(?:/(.+?))?' : '/(.+?)') : '/([^/]+?)'
+      } else {
+        return `/${escapeRegex(segment)}`
+      }
+    })
+    .join('')
 
   // dead code eliminate for browser since it's only needed
   // while generating routes-manifest
   if (typeof window === 'undefined') {
     const routeKeys: { [named: string]: string } = {}
 
-    namedParameterizedRoute = escapedRoute.replace(
-      /\/\\\[([^/]+?)\\\](?=\/|$)/g,
-      (_, $1) => {
-        const { key, repeat } = parseParameter($1)
-        // replace any non-word characters since they can break
-        // the named regex
-        const cleanedKey = key.replace(/\W/g, '')
-        routeKeys[cleanedKey] = key
-        return repeat ? `/(?<${cleanedKey}>.+?)` : `/(?<${cleanedKey}>[^/]+?)`
-      }
-    )
+    let namedParameterizedRoute = segments
+      .map((segment) => {
+        if (segment.startsWith('[') && segment.endsWith(']')) {
+          const { key, optional, repeat } = parseParameter(segment.slice(1, -1))
+          // replace any non-word characters since they can break
+          // the named regex
+          const cleanedKey = key.replace(/\W/g, '')
+          routeKeys[cleanedKey] = key
+          return repeat
+            ? optional
+              ? `(?:/(?<${cleanedKey}>.+?))?`
+              : `/(?<${cleanedKey}>.+?)`
+            : `/(?<${cleanedKey}>[^/]+?)`
+        } else {
+          return `/${escapeRegex(segment)}`
+        }
+      })
+      .join('')
 
     return {
-      re: new RegExp('^' + parameterizedRoute + '(?:/)?$', 'i'),
+      re: new RegExp(`^${parameterizedRoute}(?:/)?$`, 'i'),
       groups,
       routeKeys,
       namedRegex: `^${namedParameterizedRoute}(?:/)?$`,
@@ -73,7 +81,7 @@ export function getRouteRegex(
   }
 
   return {
-    re: new RegExp('^' + parameterizedRoute + '(?:/)?$', 'i'),
+    re: new RegExp(`^${parameterizedRoute}(?:/)?$`, 'i'),
     groups,
   }
 }
diff --git a/test/integration/prerender/test/index.test.js b/test/integration/prerender/test/index.test.js
index af5559db11838..7d0f91d4f0661 100644
--- a/test/integration/prerender/test/index.test.js
+++ b/test/integration/prerender/test/index.test.js
@@ -942,7 +942,7 @@ const runTests = (dev = false, isEmulatedServerless = false) => {
         {
           namedDataRouteRegex: `^/_next/data/${escapeRegex(
             buildId
-          )}/catchall\\-optional/(?<slug>.+?)\\.json$`,
+          )}/catchall\\-optional(?:/(?<slug>.+?))?\\.json$`,
           dataRouteRegex: normalizeRegEx(
             `^\\/_next\\/data\\/${escapeRegex(
               buildId