From 2d5d5d103c2bc8f4d9eb957354a7fe0207e8a561 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 24 May 2021 11:48:38 -0500 Subject: [PATCH] Update beforeFiles rewrites to continue --- .../lib/router/utils/resolve-rewrites.ts | 18 +++----- .../next/next-server/server/next-server.ts | 18 +++++--- test/integration/custom-routes/next.config.js | 8 ++++ test/integration/custom-routes/pages/nav.js | 8 ++++ .../custom-routes/test/index.test.js | 45 +++++++++++++++++++ 5 files changed, 80 insertions(+), 17 deletions(-) diff --git a/packages/next/next-server/lib/router/utils/resolve-rewrites.ts b/packages/next/next-server/lib/router/utils/resolve-rewrites.ts index c2bef108c98d1..b6551862abffc 100644 --- a/packages/next/next-server/lib/router/utils/resolve-rewrites.ts +++ b/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 } diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index d9fb26c72d8bb..d1f38c8ae60e8 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/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) + ) } } diff --git a/test/integration/custom-routes/next.config.js b/test/integration/custom-routes/next.config.js index 8ff2e9b06411b..f22c4c3bac9ce 100644 --- a/test/integration/custom-routes/next.config.js +++ b/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*', + }, ], } }, diff --git a/test/integration/custom-routes/pages/nav.js b/test/integration/custom-routes/pages/nav.js index 626f7e0d9f8aa..88ca61b02404c 100644 --- a/test/integration/custom-routes/pages/nav.js +++ b/test/integration/custom-routes/pages/nav.js @@ -32,5 +32,13 @@ export default () => ( to rewritten dynamic
+ + to /hello?overrideMe=1 + +
+ + to /old-blog/post-1 + +
) diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js index bbfb1a054c9d5..461774b7a799a 100644 --- a/test/integration/custom-routes/test/index.test.js +++ b/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: [], },