Update beforeFiles rewrites to continue (#25418)

This updates beforeFiles rewrites to continue instead of matching a public file/page immediately after a match, this allows all beforeFiles routes to be checked before matching the filesystem.

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.

## Documentation / Examples

- [ ] Make sure the linting passes

Author: ijjk

packages/next/next-server/lib/router/utils/resolve-rewrites.ts

@@ -101,19 +101,16 @@ export default function resolveRewrites(
   let finished = false
 
   for (let i = 0; i < rewrites.beforeFiles.length; i++) {
-    const rewrite = rewrites.beforeFiles[i]
-
-    if (handleRewrite(rewrite)) {
-      finished = true
-      break
-    }
+    // we don't end after match in beforeFiles to allow
+    // continuing through all beforeFiles rewrites
+    handleRewrite(rewrites.beforeFiles[i])
   }
+  matchedPage = pages.includes(fsPathname)
 
-  if (!pages.includes(fsPathname)) {
+  if (!matchedPage) {
     if (!finished) {
       for (let i = 0; i < rewrites.afterFiles.length; i++) {
-        const rewrite = rewrites.afterFiles[i]
-        if (handleRewrite(rewrite)) {
+        if (handleRewrite(rewrites.afterFiles[i])) {
           finished = true
           break
         }
@@ -129,8 +126,7 @@ export default function resolveRewrites(
 
     if (!finished) {
       for (let i = 0; i < rewrites.fallback.length; i++) {
-        const rewrite = rewrites.fallback[i]
-        if (handleRewrite(rewrite)) {
+        if (handleRewrite(rewrites.fallback[i])) {
           finished = true
           break
         }

packages/next/next-server/server/next-server.ts

@@ -902,11 +902,11 @@ export default class Server {
           } as Route
         })
 
-    const buildRewrite = (rewrite: Rewrite) => {
+    const buildRewrite = (rewrite: Rewrite, check = true) => {
       const rewriteRoute = getCustomRoute(rewrite, 'rewrite')
       return {
         ...rewriteRoute,
-        check: true,
+        check,
         type: rewriteRoute.type,
         name: `Rewrite route ${rewriteRoute.source}`,
         match: rewriteRoute.match,
@@ -975,11 +975,17 @@ export default class Server {
 
     if (!this.minimalMode) {
       if (Array.isArray(this.customRoutes.rewrites)) {
-        afterFiles = this.customRoutes.rewrites.map(buildRewrite)
+        afterFiles = this.customRoutes.rewrites.map((r) => buildRewrite(r))
       } else {
-        beforeFiles = this.customRoutes.rewrites.beforeFiles.map(buildRewrite)
-        afterFiles = this.customRoutes.rewrites.afterFiles.map(buildRewrite)
-        fallback = this.customRoutes.rewrites.fallback.map(buildRewrite)
+        beforeFiles = this.customRoutes.rewrites.beforeFiles.map((r) =>
+          buildRewrite(r, false)
+        )
+        afterFiles = this.customRoutes.rewrites.afterFiles.map((r) =>
+          buildRewrite(r)
+        )
+        fallback = this.customRoutes.rewrites.fallback.map((r) =>
+          buildRewrite(r)
+        )
       }
     }
 

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

@@ -190,6 +190,10 @@ module.exports = {
           ],
           destination: '/with-params?idk=:idk',
         },
+        {
+          source: '/blog/about',
+          destination: '/hello',
+        },
       ],
       beforeFiles: [
         {
@@ -202,6 +206,10 @@ module.exports = {
           ],
           destination: '/with-params?overridden=1',
         },
+        {
+          source: '/old-blog/:path*',
+          destination: '/blog/:path*',
+        },
       ],
     }
   },

test/integration/custom-routes/pages/nav.js

@@ -32,5 +32,13 @@ export default () => (
       <a id="to-rewritten-dynamic">to rewritten dynamic</a>
     </Link>
     <br />
+    <Link href="/hello?overrideMe=1">
+      <a id="to-overridden">to /hello?overrideMe=1</a>
+    </Link>
+    <br />
+    <Link href="/old-blog/about">
+      <a id="to-old-blog">to /old-blog/post-1</a>
+    </Link>
+    <br />
   </>
 )

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

@@ -38,6 +38,25 @@ let appPort
 let app
 
 const runTests = (isDev = false) => {
+  it('should continue in beforeFiles rewrites', async () => {
+    const res = await fetchViaHTTP(appPort, '/old-blog/about')
+    expect(res.status).toBe(200)
+
+    const html = await res.text()
+    const $ = cheerio.load(html)
+
+    expect($('#hello').text()).toContain('Hello')
+
+    const browser = await webdriver(appPort, '/nav')
+
+    await browser.eval('window.beforeNav = 1')
+    await browser
+      .elementByCss('#to-old-blog')
+      .click()
+      .waitForElementByCss('#hello')
+    expect(await browser.elementByCss('#hello').text()).toContain('Hello')
+  })
+
   it('should not hang when proxy rewrite fails', async () => {
     const res = await fetchViaHTTP(appPort, '/to-nowhere', undefined, {
       timeout: 5000,
@@ -781,6 +800,20 @@ const runTests = (isDev = false) => {
       overrideMe: '1',
       overridden: '1',
     })
+
+    const browser = await webdriver(appPort, '/nav')
+    await browser.eval('window.beforeNav = 1')
+    await browser.elementByCss('#to-overridden').click()
+    await browser.waitForElementByCss('#query')
+
+    expect(await browser.eval('window.next.router.pathname')).toBe(
+      '/with-params'
+    )
+    expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
+      overridden: '1',
+      overrideMe: '1',
+    })
+    expect(await browser.eval('window.beforeNav')).toBe(1)
   })
 
   it('should match has header redirect correctly', async () => {
@@ -1401,6 +1434,13 @@ const runTests = (isDev = false) => {
               regex: normalizeRegEx('^\\/hello$'),
               source: '/hello',
             },
+            {
+              destination: '/blog/:path*',
+              regex: normalizeRegEx(
+                '^\\/old-blog(?:\\/((?:[^\\/]+?)(?:\\/(?:[^\\/]+?))*))?$'
+              ),
+              source: '/old-blog/:path*',
+            },
           ],
           afterFiles: [
             {
@@ -1630,6 +1670,11 @@ const runTests = (isDev = false) => {
               regex: normalizeRegEx('^\\/has-rewrite-7$'),
               source: '/has-rewrite-7',
             },
+            {
+              destination: '/hello',
+              regex: normalizeRegEx('^\\/blog\\/about$'),
+              source: '/blog/about',
+            },
           ],
           fallback: [],
         },