From 1a3bed10ca3bb7611021184c7eb5fcbb9a9dd171 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Fri, 28 Jan 2022 14:02:14 +0100 Subject: [PATCH 1/5] Add failing test for #33703 and #23824 Add tests for the cases described in #33703 and #23824 as well as a few more. --- .../dynamic-route-interpolation/index.test.ts | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 test/e2e/dynamic-route-interpolation/index.test.ts diff --git a/test/e2e/dynamic-route-interpolation/index.test.ts b/test/e2e/dynamic-route-interpolation/index.test.ts new file mode 100644 index 000000000000..a42c15dfc485 --- /dev/null +++ b/test/e2e/dynamic-route-interpolation/index.test.ts @@ -0,0 +1,65 @@ +import { createNext } from 'e2e-utils' +import { NextInstance } from 'test/lib/next-modes/base' +import { renderViaHTTP } from 'next-test-utils' +import cheerio from 'cheerio' + +describe('Dynamic Route Interpolation', () => { + let next: NextInstance + + beforeAll(async () => { + next = await createNext({ + files: { + 'pages/blog/[slug].js': ` + export function getServerSideProps({ params }) { + return { props: { slug: params.slug } } + } + + export default function Page(props) { + return

{props.slug}

+ } + + `, + + 'pages/api/dynamic/[slug].js': ` + export default function Page(req, res) { + const { slug } = req.query + res.end('slug: ' + slug) + } + + `, + }, + dependencies: {}, + }) + }) + afterAll(() => next.destroy()) + + it('should work', async () => { + const html = await renderViaHTTP(next.url, '/blog/a') + const $ = cheerio.load(html) + expect($('#slug').text()).toBe('a') + }) + + it('should work with parameter itself', async () => { + const html = await renderViaHTTP(next.url, '/blog/[slug]') + const $ = cheerio.load(html) + expect($('#slug').text()).toBe('[slug]') + }) + + it('should work with brackets', async () => { + const html = await renderViaHTTP(next.url, '/blog/[abc]') + const $ = cheerio.load(html) + expect($('#slug').text()).toBe('[abc]') + }) + + it('should work with parameter itself in API routes', async () => { + const html = await renderViaHTTP(next.url, '/api/dynamic/[slug]') + const $ = cheerio.load(html) + expect($('#slug').text()).toBe('[slug]') + }) + + it('should work with brackets in API routes', async () => { + const html = await renderViaHTTP(next.url, '/api/dynamic/[abc]') + const $ = cheerio.load(html) + expect($('#slug').text()).toBe('[abc]') + }) +}) From 2f1dac642e79d92afa50634d17815712cc84046e Mon Sep 17 00:00:00 2001 From: Jim Fisher Date: Sun, 30 Jan 2022 13:20:39 +0100 Subject: [PATCH 2/5] Bug fix: dynamic page should not be interpreted as predefined page Fixes https://github.com/vercel/next.js/issues/23824 --- packages/next/server/base-server.ts | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index e229f0da60a6..dbb0267ec000 100644 --- a/packages/next/server/base-server.ts +++ b/packages/next/server/base-server.ts @@ -1532,15 +1532,20 @@ export default abstract class Server { delete query._nextBubbleNoFallback try { - const result = await this.findPageComponents(pathname, query) - if (result) { - try { - return await this.renderToResponseWithComponents(ctx, result) - } catch (err) { - const isNoFallbackError = err instanceof NoFallbackError + // The following guard is necessary to safely cast the URL pathname to a potential predefined page path. + // Without the guard, a request to the URL /accounts/[id] will find pages/accounts/[id].js + // and interpret it as a predefined route. + if (!isDynamicRoute(pathname)) { + const result = await this.findPageComponents(pathname, query) + if (result) { + try { + return await this.renderToResponseWithComponents(ctx, result) + } catch (err) { + const isNoFallbackError = err instanceof NoFallbackError - if (!isNoFallbackError || (isNoFallbackError && bubbleNoFallback)) { - throw err + if (!isNoFallbackError || (isNoFallbackError && bubbleNoFallback)) { + throw err + } } } } From 5d5ebe5cf45e422b829eedf328319a627828ee13 Mon Sep 17 00:00:00 2001 From: Jim Fisher Date: Mon, 31 Jan 2022 18:27:57 +0000 Subject: [PATCH 3/5] Update packages/next/server/base-server.ts Co-authored-by: JJ Kasper --- packages/next/server/base-server.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index dbb0267ec000..ca2623cb0daf 100644 --- a/packages/next/server/base-server.ts +++ b/packages/next/server/base-server.ts @@ -1532,9 +1532,8 @@ export default abstract class Server { delete query._nextBubbleNoFallback try { - // The following guard is necessary to safely cast the URL pathname to a potential predefined page path. - // Without the guard, a request to the URL /accounts/[id] will find pages/accounts/[id].js - // and interpret it as a predefined route. + // Ensure a request to the URL /accounts/[id] will be treated as a dynamic + // route correctly and not loaded immediately without parsing params. if (!isDynamicRoute(pathname)) { const result = await this.findPageComponents(pathname, query) if (result) { From c41304beadfe9bb0bf657e0fbb75054cf03e0fce Mon Sep 17 00:00:00 2001 From: Jim Fisher Date: Tue, 1 Feb 2022 08:59:54 +0000 Subject: [PATCH 4/5] equivalent fix for API routes --- packages/next/server/base-server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index dbb0267ec000..5f6143bf6b2c 100644 --- a/packages/next/server/base-server.ts +++ b/packages/next/server/base-server.ts @@ -877,7 +877,7 @@ export default abstract class Server { ): Promise { let page = pathname let params: Params | false = false - let pageFound = await this.hasPage(page) + let pageFound = !isDynamicRoute(page) && (await this.hasPage(page)) if (!pageFound && this.dynamicRoutes) { for (const dynamicRoute of this.dynamicRoutes) { From 7faf22d585ec5ca42c899a9fecc47801769ecac3 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 1 Feb 2022 20:21:45 -0600 Subject: [PATCH 5/5] update test cases --- test/e2e/dynamic-route-interpolation/index.test.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/e2e/dynamic-route-interpolation/index.test.ts b/test/e2e/dynamic-route-interpolation/index.test.ts index a42c15dfc485..441a5a5ac3bb 100644 --- a/test/e2e/dynamic-route-interpolation/index.test.ts +++ b/test/e2e/dynamic-route-interpolation/index.test.ts @@ -52,14 +52,12 @@ describe('Dynamic Route Interpolation', () => { }) it('should work with parameter itself in API routes', async () => { - const html = await renderViaHTTP(next.url, '/api/dynamic/[slug]') - const $ = cheerio.load(html) - expect($('#slug').text()).toBe('[slug]') + const text = await renderViaHTTP(next.url, '/api/dynamic/[slug]') + expect(text).toBe('slug: [slug]') }) it('should work with brackets in API routes', async () => { - const html = await renderViaHTTP(next.url, '/api/dynamic/[abc]') - const $ = cheerio.load(html) - expect($('#slug').text()).toBe('[abc]') + const text = await renderViaHTTP(next.url, '/api/dynamic/[abc]') + expect(text).toBe('slug: [abc]') }) })