Fix prefetch and some other issues with optional catch all (vercel#14400

)

Fix vercel#14290 and a couple other issues around optional catch-all that popped up after writing these tests

Closes vercel#14344

packages/next/client/page-loader.js

@@ -143,17 +143,21 @@ export default class PageLoader {
       if (
         !Object.keys(dynamicGroups).every((param) => {
           let value = dynamicMatches[param]
-          const repeat = dynamicGroups[param].repeat
+          const { repeat, optional } = dynamicGroups[param]
 
           // support single-level catch-all
           // TODO: more robust handling for user-error (passing `/`)
           if (repeat && !Array.isArray(value)) value = [value]
+          let replaced = `[${repeat ? '...' : ''}${param}]`
+          if (optional) {
+            replaced = `[${replaced}]`
+          }
 
           return (
             param in dynamicMatches &&
             // Interpolate group into data URL if present
             (interpolatedRoute = interpolatedRoute.replace(
-              `[${repeat ? '...' : ''}${param}]`,
+              replaced,
               repeat
                 ? value.map(encodeURIComponent).join('/')
                 : encodeURIComponent(value)

packages/next/next-server/lib/router/utils/route-regex.ts

@@ -4,6 +4,20 @@ function escapeRegex(str: string) {
   return str.replace(/[|\\{}()[\]^$+*?.-]/g, '\\$&')
 }
 
+function parseParameter(param: string) {
+  const optional = /^\\\[.*\\\]$/.test(param)
+  if (optional) {
+    param = param.slice(2, -2)
+  }
+  const repeat = /^(\\\.){3}/.test(param)
+  if (repeat) {
+    param = param.slice(6)
+  }
+  // Un-escape key
+  const key = param.replace(/\\([|\\{}()[\]^$+*?.-])/g, '$1')
+  return { key, repeat, optional }
+}
+
 export function getRouteRegex(
   normalizedRoute: string
 ): {
@@ -25,21 +39,9 @@ export function getRouteRegex(
   const parameterizedRoute = escapedRoute.replace(
     /\/\\\[([^/]+?)\\\](?=\/|$)/g,
     (_, $1) => {
-      const isOptional = /^\\\[.*\\\]$/.test($1)
-      if (isOptional) {
-        $1 = $1.slice(2, -2)
-      }
-      const isCatchAll = /^(\\\.){3}/.test($1)
-      if (isCatchAll) {
-        $1 = $1.slice(6)
-      }
-      groups[
-        $1
-          // Un-escape key
-          .replace(/\\([|\\{}()[\]^$+*?.-])/g, '$1')
-        // eslint-disable-next-line no-sequences
-      ] = { pos: groupIndex++, repeat: isCatchAll, optional: isOptional }
-      return isCatchAll ? (isOptional ? '(?:/(.+?))?' : '/(.+?)') : '/([^/]+?)'
+      const { key, optional, repeat } = parseParameter($1)
+      groups[key] = { pos: groupIndex++, repeat, optional }
+      return repeat ? (optional ? '(?:/(.+?))?' : '/(.+?)') : '/([^/]+?)'
     }
   )
 
@@ -53,21 +55,12 @@ export function getRouteRegex(
     namedParameterizedRoute = escapedRoute.replace(
       /\/\\\[([^/]+?)\\\](?=\/|$)/g,
       (_, $1) => {
-        const isCatchAll = /^(\\\.){3}/.test($1)
-        const key = $1
-          // Un-escape key
-          .replace(/\\([|\\{}()[\]^$+*?.-])/g, '$1')
-          .replace(/^\.{3}/, '')
-
+        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 isCatchAll
-          ? `/(?<${cleanedKey}>.+?)`
-          : `/(?<${cleanedKey}>[^/]+?)`
+        return repeat ? `/(?<${cleanedKey}>.+?)` : `/(?<${cleanedKey}>[^/]+?)`
       }
     )
 

test/integration/prerender/next.config.js

@@ -1,5 +1,6 @@
 module.exports = {
   experimental: {
+    optionalCatchAll: true,
     rewrites() {
       return [
         {

test/integration/prerender/pages/catchall-optional/[[...slug]].js

@@ -0,0 +1,29 @@
+import Link from 'next/link'
+
+export async function getStaticProps({ params: { slug } }) {
+  return {
+    props: {
+      slug: slug || [],
+    },
+  }
+}
+
+export async function getStaticPaths() {
+  return {
+    paths: [{ params: { slug: [] } }, { params: { slug: ['value'] } }],
+    fallback: false,
+  }
+}
+
+export default ({ slug }) => {
+  // Important to not check for `slug` existence (testing that build does not
+  // render fallback version and error)
+  return (
+    <>
+      <p id="catchall">Catch all: [{slug.join(', ')}]</p>
+      <Link href="/">
+        <a id="home">to home</a>
+      </Link>
+    </>
+  )
+}

test/integration/prerender/pages/index.js

@@ -47,6 +47,13 @@ const Page = ({ world, time }) => {
       <Link href="/catchall/[...slug]" as="/catchall/first">
         <a id="to-catchall">to catchall</a>
       </Link>
+      <br />
+      <Link href="/catchall-optional/[[...slug]]" as="/catchall-optional">
+        <a id="catchall-optional-root">to optional catchall root</a>
+      </Link>
+      <Link href="/catchall-optional/[[...slug]]" as="/catchall-optional/value">
+        <a id="catchall-optional-value">to optional catchall page /value</a>
+      </Link>
     </>
   )
 }

test/integration/prerender/test/index.test.js

@@ -116,6 +116,16 @@ const expectedManifestRoutes = () => ({
     initialRevalidateSeconds: 1,
     srcRoute: '/catchall-explicit/[...slug]',
   },
+  '/catchall-optional': {
+    dataRoute: `/_next/data/${buildId}/catchall-optional.json`,
+    initialRevalidateSeconds: false,
+    srcRoute: '/catchall-optional/[[...slug]]',
+  },
+  '/catchall-optional/value': {
+    dataRoute: `/_next/data/${buildId}/catchall-optional/value.json`,
+    initialRevalidateSeconds: false,
+    srcRoute: '/catchall-optional/[[...slug]]',
+  },
   '/another': {
     dataRoute: `/_next/data/${buildId}/another.json`,
     initialRevalidateSeconds: 1,
@@ -270,6 +280,28 @@ const navigateTest = (dev = false) => {
     await browser.elementByCss('#home').click()
     await browser.waitForElementByCss('#comment-1')
 
+    // go to /catchall-optional
+    await browser.elementByCss('#catchall-optional-root').click()
+    await browser.waitForElementByCss('#home')
+    text = await browser.elementByCss('p').text()
+    expect(text).toMatch(/Catch all: \[\]/)
+    expect(await browser.eval('window.didTransition')).toBe(1)
+
+    // go to /
+    await browser.elementByCss('#home').click()
+    await browser.waitForElementByCss('#comment-1')
+
+    // go to /catchall-optional/value
+    await browser.elementByCss('#catchall-optional-value').click()
+    await browser.waitForElementByCss('#home')
+    text = await browser.elementByCss('p').text()
+    expect(text).toMatch(/Catch all: \[value\]/)
+    expect(await browser.eval('window.didTransition')).toBe(1)
+
+    // go to /
+    await browser.elementByCss('#home').click()
+    await browser.waitForElementByCss('#comment-1')
+
     // go to /blog/post-1/comment-1
     await browser.elementByCss('#comment-1').click()
     await browser.waitForElementByCss('#home')
@@ -907,6 +939,20 @@ const runTests = (dev = false, isEmulatedServerless = false) => {
             slug: 'slug',
           },
         },
+        {
+          namedDataRouteRegex: `^/_next/data/${escapeRegex(
+            buildId
+          )}/catchall\\-optional/(?<slug>.+?)\\.json$`,
+          dataRouteRegex: normalizeRegEx(
+            `^\\/_next\\/data\\/${escapeRegex(
+              buildId
+            )}\\/catchall\\-optional(?:\\/(.+?))?\\.json$`
+          ),
+          page: '/catchall-optional/[[...slug]]',
+          routeKeys: {
+            slug: 'slug',
+          },
+        },
         {
           dataRouteRegex: normalizeRegEx(
             `^\\/_next\\/data\\/${escapeRegex(
@@ -1054,6 +1100,7 @@ const runTests = (dev = false, isEmulatedServerless = false) => {
             `^\\/user\\/([^\\/]+?)\\/profile(?:\\/)?$`
           ),
         },
+
         '/catchall/[...slug]': {
           fallback: '/catchall/[...slug].html',
           routeRegex: normalizeRegEx('^\\/catchall\\/(.+?)(?:\\/)?$'),
@@ -1062,6 +1109,16 @@ const runTests = (dev = false, isEmulatedServerless = false) => {
             `^\\/_next\\/data\\/${escapedBuildId}\\/catchall\\/(.+?)\\.json$`
           ),
         },
+        '/catchall-optional/[[...slug]]': {
+          dataRoute: `/_next/data/${buildId}/catchall-optional/[[...slug]].json`,
+          dataRouteRegex: normalizeRegEx(
+            `^\\/_next\\/data\\/${escapedBuildId}\\/catchall\\-optional(?:\\/(.+?))?\\.json$`
+          ),
+          fallback: false,
+          routeRegex: normalizeRegEx(
+            '^\\/catchall\\-optional(?:\\/(.+?))?(?:\\/)?$'
+          ),
+        },
         '/catchall-explicit/[...slug]': {
           dataRoute: `/_next/data/${buildId}/catchall-explicit/[...slug].json`,
           dataRouteRegex: normalizeRegEx(
@@ -1213,6 +1270,7 @@ describe('SSG Prerender', () => {
         `
         module.exports = {
           experimental: {
+            optionalCatchAll: true,
             rewrites() {
               return [
                 {
@@ -1253,7 +1311,7 @@ describe('SSG Prerender', () => {
         nextConfig,
         // we set cpus to 1 so that we make sure the requests
         // aren't being cached at the jest-worker level
-        `module.exports = { experimental: { cpus: 1 } }`,
+        `module.exports = { experimental: { optionalCatchAll: true, cpus: 1 } }`,
         'utf8'
       )
       await fs.remove(join(appDir, '.next'))
@@ -1321,6 +1379,7 @@ describe('SSG Prerender', () => {
         `module.exports = {
           target: 'serverless',
           experimental: {
+            optionalCatchAll: true,
             rewrites() {
               return [
                 {
@@ -1430,7 +1489,7 @@ describe('SSG Prerender', () => {
       origConfig = await fs.readFile(nextConfig, 'utf8')
       await fs.writeFile(
         nextConfig,
-        `module.exports = { target: 'experimental-serverless-trace' }`,
+        `module.exports = { target: 'experimental-serverless-trace', experimental: { optionalCatchAll: true } }`,
         'utf8'
       )
       await fs.writeFile(
@@ -1516,6 +1575,7 @@ describe('SSG Prerender', () => {
       await fs.writeFile(
         nextConfig,
         `module.exports = {
+          experimental: { optionalCatchAll: true },
           exportTrailingSlash: true,
           exportPathMap: function(defaultPathMap) {
             if (defaultPathMap['/blog/[post]']) {