From 68e1e677a4b700ff24903f2927a62937a36649d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Fri, 18 Nov 2022 13:25:21 +0100 Subject: [PATCH 01/15] Add new error link --- errors/app-dir-href-interpolation.md | 30 ++++++++++++++++++++++++++++ errors/manifest.json | 4 ++++ 2 files changed, 34 insertions(+) create mode 100644 errors/app-dir-href-interpolation.md diff --git a/errors/app-dir-href-interpolation.md b/errors/app-dir-href-interpolation.md new file mode 100644 index 000000000000..fe6465d83845 --- /dev/null +++ b/errors/app-dir-href-interpolation.md @@ -0,0 +1,30 @@ +# `href` interpolation is not supported in the `/app` directory + +#### Why This Error Occurred + +You have tried to use `href` interpolation with `next/link` in the `/app` directory. This is not supported. + +#### Possible Ways to Fix It + +**Before** + +```jsx + + link + +``` + +**After** + +```jsx +link +``` + +### Useful Links + +[`next/link` documentation](https://beta.nextjs.org/docs/api-reference/components/link#href) diff --git a/errors/manifest.json b/errors/manifest.json index 9be351338750..c1ff3deb09e5 100644 --- a/errors/manifest.json +++ b/errors/manifest.json @@ -761,6 +761,10 @@ { "title": "next-router-not-mounted", "path": "/errors/next-router-not-mounted.md" + }, + { + "title": "app-dir-href-interpolation", + "path": "/errors/app-dir-href-interpolation.md" } ] } From a52276b2a3ee6f6f71ef44b6bd8de2407ae14f04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Fri, 18 Nov 2022 13:26:37 +0100 Subject: [PATCH 02/15] Move getSegmentParam to utils --- packages/next/server/app-render.tsx | 33 +---------------------------- packages/next/server/utils.ts | 32 ++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index ce51949a97bb..926bf8292858 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -43,6 +43,7 @@ import { NEXT_ROUTER_STATE_TREE, RSC, } from '../client/components/app-router-headers' +import { type DynamicParamTypes, getSegmentParam } from './utils' const isEdgeRuntime = process.env.NEXT_RUNTIME === 'edge' @@ -443,7 +444,6 @@ function createServerComponentRenderer( } } -type DynamicParamTypes = 'catchall' | 'optional-catchall' | 'dynamic' // c = catchall // oc = optional catchall // d = dynamic @@ -539,37 +539,6 @@ export type ChildProp = { segment: Segment } -/** - * Parse dynamic route segment to type of parameter - */ -function getSegmentParam(segment: string): { - param: string - type: DynamicParamTypes -} | null { - if (segment.startsWith('[[...') && segment.endsWith(']]')) { - return { - type: 'optional-catchall', - param: segment.slice(5, -2), - } - } - - if (segment.startsWith('[...') && segment.endsWith(']')) { - return { - type: 'catchall', - param: segment.slice(4, -1), - } - } - - if (segment.startsWith('[') && segment.endsWith(']')) { - return { - type: 'dynamic', - param: segment.slice(1, -1), - } - } - - return null -} - /** * Get inline tags based on server CSS manifest. Only used when rendering to HTML. */ diff --git a/packages/next/server/utils.ts b/packages/next/server/utils.ts index c8c90fdd9789..d76195371a0e 100644 --- a/packages/next/server/utils.ts +++ b/packages/next/server/utils.ts @@ -14,3 +14,35 @@ export function cleanAmpPath(pathname: string): string { pathname = pathname.replace(/\?$/, '') return pathname } + +export type DynamicParamTypes = 'catchall' | 'optional-catchall' | 'dynamic' +/** + * Parse dynamic route segment to type of parameter + */ +export function getSegmentParam(segment: string): { + param: string + type: DynamicParamTypes +} | null { + if (segment.startsWith('[[...') && segment.endsWith(']]')) { + return { + type: 'optional-catchall', + param: segment.slice(5, -2), + } + } + + if (segment.startsWith('[...') && segment.endsWith(']')) { + return { + type: 'catchall', + param: segment.slice(4, -1), + } + } + + if (segment.startsWith('[') && segment.endsWith(']')) { + return { + type: 'dynamic', + param: segment.slice(1, -1), + } + } + + return null +} From 8c5f10ce5df92854e9ffe930bfe1eefd37d26afc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Fri, 18 Nov 2022 13:31:23 +0100 Subject: [PATCH 03/15] Error in next/link if in dev with the app router when href interpolation is found --- packages/next/client/link.tsx | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 54b51d8a51bb..2dcda00ff236 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -18,6 +18,7 @@ import { import { useIntersection } from './use-intersection' import { getDomainLocale } from './get-domain-locale' import { addBasePath } from './add-base-path' +import { getSegmentParam } from '../server/utils' type Url = string | UrlObject type RequiredKeys = { @@ -397,6 +398,31 @@ const Link = React.forwardRef( // We're in the app directory if there is no pages router. const isAppRouter = !pagesRouter + if (process.env.NODE_ENV !== 'production') { + if ( + isAppRouter && + typeof hrefProp === 'object' && + typeof hrefProp.pathname === 'string' && + hrefProp.query && + typeof hrefProp.query === 'object' + ) { + const { pathname, query } = hrefProp + + const dynamicSegments: string[] = pathname + .split('/') + .map((segment: string) => getSegmentParam(segment)?.param) + .filter(Boolean) as string[] + + for (const dynamicSegment of dynamicSegments) { + if (query[dynamicSegment]) { + throw new Error( + `The \`/app\` router does not support \`href\` interpolation, found pathname \`${pathname}\` using query \`${dynamicSegment}\`. Read more: https://nextjs.org/docs/messages/app-dir-href-interpolation` + ) + } + } + } + } + const { href, as } = React.useMemo(() => { if (!pagesRouter) { const resolvedHref = formatStringOrUrl(hrefProp) From 1525ffdb0acfbb2b4af85d86ba3685ea5d530802 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Fri, 18 Nov 2022 13:31:54 +0100 Subject: [PATCH 04/15] Add e2e tests --- test/e2e/app-dir/href-interpolation.test.ts | 58 +++++++++++++++++++ .../href-interpolation/app/[slug]/page.js | 14 +++++ .../app-dir/href-interpolation/app/layout.js | 10 ++++ .../app-dir/href-interpolation/app/page.js | 15 +++++ .../app-dir/href-interpolation/next.config.js | 5 ++ 5 files changed, 102 insertions(+) create mode 100644 test/e2e/app-dir/href-interpolation.test.ts create mode 100644 test/e2e/app-dir/href-interpolation/app/[slug]/page.js create mode 100644 test/e2e/app-dir/href-interpolation/app/layout.js create mode 100644 test/e2e/app-dir/href-interpolation/app/page.js create mode 100644 test/e2e/app-dir/href-interpolation/next.config.js diff --git a/test/e2e/app-dir/href-interpolation.test.ts b/test/e2e/app-dir/href-interpolation.test.ts new file mode 100644 index 000000000000..2342c5f92404 --- /dev/null +++ b/test/e2e/app-dir/href-interpolation.test.ts @@ -0,0 +1,58 @@ +import { createNext, FileRef } from 'e2e-utils' +import { NextInstance } from 'test/lib/next-modes/base' +import { getRedboxDescription, hasRedbox } from 'next-test-utils' +import path from 'path' +import webdriver from 'next-webdriver' + +describe('href-interpolation', () => { + const isDev = (global as any).isNextDev + if ((global as any).isNextDeploy) { + it('should skip next deploy for now', () => {}) + return + } + + let next: NextInstance + + beforeAll(async () => { + next = await createNext({ + files: new FileRef(path.join(__dirname, 'href-interpolation')), + dependencies: { + react: 'experimental', + 'react-dom': 'experimental', + }, + }) + }) + afterAll(() => next.destroy()) + + if (isDev) { + it('should error when using href interpolation in app dir', async () => { + const browser = await webdriver(next.url, '/') + + // Error should show up + expect(await hasRedbox(browser, true)).toBeTrue() + expect(await getRedboxDescription(browser)).toMatchInlineSnapshot( + `"Error: The \`/app\` router does not support \`href\` interpolation, found pathname \`/[slug]\` using query \`slug\`. Read more: https://nextjs.org/docs/messages/app-dir-href-interpolation"` + ) + + // Fix error + const pageContent = await next.readFile('app/page.js') + await next.patchFile( + 'app/page.js', + pageContent.replace("pathname: '/[slug]'", "pathname: '/slug'") + ) + expect(await browser.waitForElementByCss('#link').text()).toBe('to slug') + + // Navigate to new page + await browser.elementByCss('#link').click() + expect(await browser.waitForElementByCss('#pathname').text()).toBe( + '/slug' + ) + expect(await browser.elementByCss('#slug').text()).toBe('1') + }) + } else { + it('should not error in prod', async () => { + const browser = await webdriver(next.url, '/') + expect(await browser.elementByCss('#link').text()).toBe('to slug') + }) + } +}) diff --git a/test/e2e/app-dir/href-interpolation/app/[slug]/page.js b/test/e2e/app-dir/href-interpolation/app/[slug]/page.js new file mode 100644 index 000000000000..911892225aff --- /dev/null +++ b/test/e2e/app-dir/href-interpolation/app/[slug]/page.js @@ -0,0 +1,14 @@ +'use client' +import { usePathname, useSearchParams } from 'next/navigation' + +export default function Page() { + const pathname = usePathname() + const searchParams = useSearchParams() + + return ( + <> +

{pathname}

+

{searchParams.get('slug')}

+ + ) +} diff --git a/test/e2e/app-dir/href-interpolation/app/layout.js b/test/e2e/app-dir/href-interpolation/app/layout.js new file mode 100644 index 000000000000..c84b681925eb --- /dev/null +++ b/test/e2e/app-dir/href-interpolation/app/layout.js @@ -0,0 +1,10 @@ +export default function Root({ children }) { + return ( + + + Hello World + + {children} + + ) +} diff --git a/test/e2e/app-dir/href-interpolation/app/page.js b/test/e2e/app-dir/href-interpolation/app/page.js new file mode 100644 index 000000000000..a428f9920eb7 --- /dev/null +++ b/test/e2e/app-dir/href-interpolation/app/page.js @@ -0,0 +1,15 @@ +import Link from 'next/link' + +export default function HomePage() { + return ( + + to slug + + ) +} diff --git a/test/e2e/app-dir/href-interpolation/next.config.js b/test/e2e/app-dir/href-interpolation/next.config.js new file mode 100644 index 000000000000..cfa3ac3d7aa9 --- /dev/null +++ b/test/e2e/app-dir/href-interpolation/next.config.js @@ -0,0 +1,5 @@ +module.exports = { + experimental: { + appDir: true, + }, +} From d6065655fbd77df4523214232ece88d782f0def6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Mon, 21 Nov 2022 09:20:26 +0100 Subject: [PATCH 05/15] cleanup --- packages/next/client/link.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 2dcda00ff236..b089d3dbb84e 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -408,7 +408,7 @@ const Link = React.forwardRef( ) { const { pathname, query } = hrefProp - const dynamicSegments: string[] = pathname + const dynamicSegments = pathname .split('/') .map((segment: string) => getSegmentParam(segment)?.param) .filter(Boolean) as string[] From 388b8bca5c9cccab9696e8a622110b2f81cfa940 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Mon, 21 Nov 2022 10:52:11 +0100 Subject: [PATCH 06/15] Move back getSegmentParam to app-render --- packages/next/server/app-render.tsx | 32 ++++++++++++++++++++++++++++- packages/next/server/utils.ts | 32 ----------------------------- 2 files changed, 31 insertions(+), 33 deletions(-) diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index e6aff904274b..9f36fd4bbf01 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -44,7 +44,6 @@ import { RSC, } from '../client/components/app-router-headers' import type { StaticGenerationStore } from '../client/components/static-generation-async-storage' -import { type DynamicParamTypes, getSegmentParam } from './utils' const isEdgeRuntime = process.env.NEXT_RUNTIME === 'edge' @@ -438,10 +437,41 @@ function createServerComponentRenderer( } } +type DynamicParamTypes = 'catchall' | 'optional-catchall' | 'dynamic' // c = catchall // oc = optional catchall // d = dynamic export type DynamicParamTypesShort = 'c' | 'oc' | 'd' +/** + * Parse dynamic route segment to type of parameter + */ +function getSegmentParam(segment: string): { + param: string + type: DynamicParamTypes +} | null { + if (segment.startsWith('[[...') && segment.endsWith(']]')) { + return { + type: 'optional-catchall', + param: segment.slice(5, -2), + } + } + + if (segment.startsWith('[...') && segment.endsWith(']')) { + return { + type: 'catchall', + param: segment.slice(4, -1), + } + } + + if (segment.startsWith('[') && segment.endsWith(']')) { + return { + type: 'dynamic', + param: segment.slice(1, -1), + } + } + + return null +} /** * Shorten the dynamic param in order to make it smaller when transmitted to the browser. diff --git a/packages/next/server/utils.ts b/packages/next/server/utils.ts index d76195371a0e..c8c90fdd9789 100644 --- a/packages/next/server/utils.ts +++ b/packages/next/server/utils.ts @@ -14,35 +14,3 @@ export function cleanAmpPath(pathname: string): string { pathname = pathname.replace(/\?$/, '') return pathname } - -export type DynamicParamTypes = 'catchall' | 'optional-catchall' | 'dynamic' -/** - * Parse dynamic route segment to type of parameter - */ -export function getSegmentParam(segment: string): { - param: string - type: DynamicParamTypes -} | null { - if (segment.startsWith('[[...') && segment.endsWith(']]')) { - return { - type: 'optional-catchall', - param: segment.slice(5, -2), - } - } - - if (segment.startsWith('[...') && segment.endsWith(']')) { - return { - type: 'catchall', - param: segment.slice(4, -1), - } - } - - if (segment.startsWith('[') && segment.endsWith(']')) { - return { - type: 'dynamic', - param: segment.slice(1, -1), - } - } - - return null -} From 9743bdae6c1910baafdf0c3d3bfa2497f77d3a6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Mon, 21 Nov 2022 11:24:52 +0100 Subject: [PATCH 07/15] Update error --- ...{app-dir-href-interpolation.md => app-dir-dynamic-href.md} | 4 ++-- errors/manifest.json | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) rename errors/{app-dir-href-interpolation.md => app-dir-dynamic-href.md} (66%) diff --git a/errors/app-dir-href-interpolation.md b/errors/app-dir-dynamic-href.md similarity index 66% rename from errors/app-dir-href-interpolation.md rename to errors/app-dir-dynamic-href.md index fe6465d83845..b47f5736b690 100644 --- a/errors/app-dir-href-interpolation.md +++ b/errors/app-dir-dynamic-href.md @@ -1,8 +1,8 @@ -# `href` interpolation is not supported in the `/app` directory +# Dynamic `href` is not supported in the `/app` directory #### Why This Error Occurred -You have tried to use `href` interpolation with `next/link` in the `/app` directory. This is not supported. +You have tried to use a dynamic `href` with `next/link` in the `/app` directory. This is not supported. #### Possible Ways to Fix It diff --git a/errors/manifest.json b/errors/manifest.json index c1ff3deb09e5..908d623ad256 100644 --- a/errors/manifest.json +++ b/errors/manifest.json @@ -763,8 +763,8 @@ "path": "/errors/next-router-not-mounted.md" }, { - "title": "app-dir-href-interpolation", - "path": "/errors/app-dir-href-interpolation.md" + "title": "app-dir-dynamic-href", + "path": "/errors/app-dir-dynamic-href.md" } ] } From e2ce1f3c3ede04e2ff1aa7b792e4c78a33d64565 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Mon, 21 Nov 2022 11:25:12 +0100 Subject: [PATCH 08/15] Update next/link --- packages/next/client/link.tsx | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index b089d3dbb84e..ab2a9f170ebd 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -18,7 +18,6 @@ import { import { useIntersection } from './use-intersection' import { getDomainLocale } from './get-domain-locale' import { addBasePath } from './add-base-path' -import { getSegmentParam } from '../server/utils' type Url = string | UrlObject type RequiredKeys = { @@ -402,23 +401,18 @@ const Link = React.forwardRef( if ( isAppRouter && typeof hrefProp === 'object' && - typeof hrefProp.pathname === 'string' && - hrefProp.query && - typeof hrefProp.query === 'object' + typeof hrefProp.pathname === 'string' ) { - const { pathname, query } = hrefProp + const { pathname } = hrefProp - const dynamicSegments = pathname + const hasDynamicSegment = pathname .split('/') - .map((segment: string) => getSegmentParam(segment)?.param) - .filter(Boolean) as string[] + .some((segment) => segment.startsWith('[') && segment.endsWith(']')) - for (const dynamicSegment of dynamicSegments) { - if (query[dynamicSegment]) { - throw new Error( - `The \`/app\` router does not support \`href\` interpolation, found pathname \`${pathname}\` using query \`${dynamicSegment}\`. Read more: https://nextjs.org/docs/messages/app-dir-href-interpolation` - ) - } + if (hasDynamicSegment) { + throw new Error( + `Dynamic href \`${pathname}\` found while using the \`/app\` router, this is not supported. Read more: https://nextjs.org/docs/messages/app-dir-dynamic-href` + ) } } } From 4263c7f17f59fee11bc4a9bcdd63cda0eda85088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Mon, 21 Nov 2022 11:25:21 +0100 Subject: [PATCH 09/15] Update tests --- .../{href-interpolation.test.ts => dynamic-href.test.ts} | 6 +++--- .../{href-interpolation => dynamic-href}/app/[slug]/page.js | 0 .../{href-interpolation => dynamic-href}/app/layout.js | 0 .../{href-interpolation => dynamic-href}/app/page.js | 0 .../{href-interpolation => dynamic-href}/next.config.js | 0 5 files changed, 3 insertions(+), 3 deletions(-) rename test/e2e/app-dir/{href-interpolation.test.ts => dynamic-href.test.ts} (84%) rename test/e2e/app-dir/{href-interpolation => dynamic-href}/app/[slug]/page.js (100%) rename test/e2e/app-dir/{href-interpolation => dynamic-href}/app/layout.js (100%) rename test/e2e/app-dir/{href-interpolation => dynamic-href}/app/page.js (100%) rename test/e2e/app-dir/{href-interpolation => dynamic-href}/next.config.js (100%) diff --git a/test/e2e/app-dir/href-interpolation.test.ts b/test/e2e/app-dir/dynamic-href.test.ts similarity index 84% rename from test/e2e/app-dir/href-interpolation.test.ts rename to test/e2e/app-dir/dynamic-href.test.ts index 2342c5f92404..41c97ed0bfb7 100644 --- a/test/e2e/app-dir/href-interpolation.test.ts +++ b/test/e2e/app-dir/dynamic-href.test.ts @@ -4,7 +4,7 @@ import { getRedboxDescription, hasRedbox } from 'next-test-utils' import path from 'path' import webdriver from 'next-webdriver' -describe('href-interpolation', () => { +describe('dynamic-href', () => { const isDev = (global as any).isNextDev if ((global as any).isNextDeploy) { it('should skip next deploy for now', () => {}) @@ -15,7 +15,7 @@ describe('href-interpolation', () => { beforeAll(async () => { next = await createNext({ - files: new FileRef(path.join(__dirname, 'href-interpolation')), + files: new FileRef(path.join(__dirname, 'dynamic-href')), dependencies: { react: 'experimental', 'react-dom': 'experimental', @@ -31,7 +31,7 @@ describe('href-interpolation', () => { // Error should show up expect(await hasRedbox(browser, true)).toBeTrue() expect(await getRedboxDescription(browser)).toMatchInlineSnapshot( - `"Error: The \`/app\` router does not support \`href\` interpolation, found pathname \`/[slug]\` using query \`slug\`. Read more: https://nextjs.org/docs/messages/app-dir-href-interpolation"` + `"Error: Dynamic href \`/[slug]\` found while using the \`/app\` router, this is not supported. Read more: https://nextjs.org/docs/messages/app-dir-dynamic-href"` ) // Fix error diff --git a/test/e2e/app-dir/href-interpolation/app/[slug]/page.js b/test/e2e/app-dir/dynamic-href/app/[slug]/page.js similarity index 100% rename from test/e2e/app-dir/href-interpolation/app/[slug]/page.js rename to test/e2e/app-dir/dynamic-href/app/[slug]/page.js diff --git a/test/e2e/app-dir/href-interpolation/app/layout.js b/test/e2e/app-dir/dynamic-href/app/layout.js similarity index 100% rename from test/e2e/app-dir/href-interpolation/app/layout.js rename to test/e2e/app-dir/dynamic-href/app/layout.js diff --git a/test/e2e/app-dir/href-interpolation/app/page.js b/test/e2e/app-dir/dynamic-href/app/page.js similarity index 100% rename from test/e2e/app-dir/href-interpolation/app/page.js rename to test/e2e/app-dir/dynamic-href/app/page.js diff --git a/test/e2e/app-dir/href-interpolation/next.config.js b/test/e2e/app-dir/dynamic-href/next.config.js similarity index 100% rename from test/e2e/app-dir/href-interpolation/next.config.js rename to test/e2e/app-dir/dynamic-href/next.config.js From 227b4f4169217590ed55c5e470e338350bf94bcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Mon, 21 Nov 2022 11:37:32 +0100 Subject: [PATCH 10/15] move function --- packages/next/server/app-render.tsx | 61 +++++++++++++++-------------- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index 9f36fd4bbf01..4a6d4b55b3d0 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -442,36 +442,6 @@ type DynamicParamTypes = 'catchall' | 'optional-catchall' | 'dynamic' // oc = optional catchall // d = dynamic export type DynamicParamTypesShort = 'c' | 'oc' | 'd' -/** - * Parse dynamic route segment to type of parameter - */ -function getSegmentParam(segment: string): { - param: string - type: DynamicParamTypes -} | null { - if (segment.startsWith('[[...') && segment.endsWith(']]')) { - return { - type: 'optional-catchall', - param: segment.slice(5, -2), - } - } - - if (segment.startsWith('[...') && segment.endsWith(']')) { - return { - type: 'catchall', - param: segment.slice(4, -1), - } - } - - if (segment.startsWith('[') && segment.endsWith(']')) { - return { - type: 'dynamic', - param: segment.slice(1, -1), - } - } - - return null -} /** * Shorten the dynamic param in order to make it smaller when transmitted to the browser. @@ -563,6 +533,37 @@ export type ChildProp = { segment: Segment } +/** + * Parse dynamic route segment to type of parameter + */ +function getSegmentParam(segment: string): { + param: string + type: DynamicParamTypes +} | null { + if (segment.startsWith('[[...') && segment.endsWith(']]')) { + return { + type: 'optional-catchall', + param: segment.slice(5, -2), + } + } + + if (segment.startsWith('[...') && segment.endsWith(']')) { + return { + type: 'catchall', + param: segment.slice(4, -1), + } + } + + if (segment.startsWith('[') && segment.endsWith(']')) { + return { + type: 'dynamic', + param: segment.slice(1, -1), + } + } + + return null +} + /** * Get inline tags based on server CSS manifest. Only used when rendering to HTML. */ From 0753727500eb6e3b69bf8c3661af78550990bb8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Mon, 21 Nov 2022 13:18:21 +0100 Subject: [PATCH 11/15] Also check for dynamic string in link --- packages/next/client/link.tsx | 36 ++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index ab2a9f170ebd..ee370ddd5766 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -398,21 +398,27 @@ const Link = React.forwardRef( const isAppRouter = !pagesRouter if (process.env.NODE_ENV !== 'production') { - if ( - isAppRouter && - typeof hrefProp === 'object' && - typeof hrefProp.pathname === 'string' - ) { - const { pathname } = hrefProp - - const hasDynamicSegment = pathname - .split('/') - .some((segment) => segment.startsWith('[') && segment.endsWith(']')) - - if (hasDynamicSegment) { - throw new Error( - `Dynamic href \`${pathname}\` found while using the \`/app\` router, this is not supported. Read more: https://nextjs.org/docs/messages/app-dir-dynamic-href` - ) + if (isAppRouter) { + let href: string | undefined + if (typeof hrefProp === 'string') { + href = hrefProp + } else if ( + typeof hrefProp === 'object' && + typeof hrefProp.pathname === 'string' + ) { + href = hrefProp.pathname + } + + if (href) { + const hasDynamicSegment = href + .split('/') + .some((segment) => segment.startsWith('[') && segment.endsWith(']')) + + if (hasDynamicSegment) { + throw new Error( + `Dynamic href \`${href}\` found in while using the \`/app\` router, this is not supported. Read more: https://nextjs.org/docs/messages/app-dir-dynamic-href` + ) + } } } } From cb7479c788cbdf74c1b573b79bc27743eea4c57f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Mon, 21 Nov 2022 13:18:39 +0100 Subject: [PATCH 12/15] Update error --- errors/app-dir-dynamic-href.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/errors/app-dir-dynamic-href.md b/errors/app-dir-dynamic-href.md index b47f5736b690..ec94ed04c6da 100644 --- a/errors/app-dir-dynamic-href.md +++ b/errors/app-dir-dynamic-href.md @@ -19,6 +19,12 @@ You have tried to use a dynamic `href` with `next/link` in the `/app` directory. ``` +Or + +```jsx +link +``` + **After** ```jsx From e310e41d10c43df48845f603603ab152c5b43ef0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Mon, 21 Nov 2022 13:19:01 +0100 Subject: [PATCH 13/15] Update tests --- test/e2e/app-dir/dynamic-href.test.ts | 35 ++++++++++++++----- .../app/{ => object}/[slug]/page.js | 0 .../dynamic-href/app/{ => object}/page.js | 2 +- .../app-dir/dynamic-href/app/string/page.js | 9 +++++ 4 files changed, 36 insertions(+), 10 deletions(-) rename test/e2e/app-dir/dynamic-href/app/{ => object}/[slug]/page.js (100%) rename test/e2e/app-dir/dynamic-href/app/{ => object}/page.js (84%) create mode 100644 test/e2e/app-dir/dynamic-href/app/string/page.js diff --git a/test/e2e/app-dir/dynamic-href.test.ts b/test/e2e/app-dir/dynamic-href.test.ts index 41c97ed0bfb7..6615538c55e2 100644 --- a/test/e2e/app-dir/dynamic-href.test.ts +++ b/test/e2e/app-dir/dynamic-href.test.ts @@ -25,33 +25,50 @@ describe('dynamic-href', () => { afterAll(() => next.destroy()) if (isDev) { - it('should error when using href interpolation in app dir', async () => { - const browser = await webdriver(next.url, '/') + it('should error when using dynamic href.pathname in app dir', async () => { + const browser = await webdriver(next.url, '/object') // Error should show up expect(await hasRedbox(browser, true)).toBeTrue() expect(await getRedboxDescription(browser)).toMatchInlineSnapshot( - `"Error: Dynamic href \`/[slug]\` found while using the \`/app\` router, this is not supported. Read more: https://nextjs.org/docs/messages/app-dir-dynamic-href"` + `"Error: Dynamic href \`/object/[slug]\` found in while using the \`/app\` router, this is not supported. Read more: https://nextjs.org/docs/messages/app-dir-dynamic-href"` ) // Fix error - const pageContent = await next.readFile('app/page.js') + const pageContent = await next.readFile('app/object/page.js') await next.patchFile( - 'app/page.js', - pageContent.replace("pathname: '/[slug]'", "pathname: '/slug'") + 'app/object/page.js', + pageContent.replace( + "pathname: '/object/[slug]'", + "pathname: '/object/slug'" + ) ) expect(await browser.waitForElementByCss('#link').text()).toBe('to slug') // Navigate to new page await browser.elementByCss('#link').click() expect(await browser.waitForElementByCss('#pathname').text()).toBe( - '/slug' + '/object/slug' ) expect(await browser.elementByCss('#slug').text()).toBe('1') }) + + it('should error when using dynamic href in app dir', async () => { + const browser = await webdriver(next.url, '/string') + + // Error should show up + expect(await hasRedbox(browser, true)).toBeTrue() + expect(await getRedboxDescription(browser)).toMatchInlineSnapshot( + `"Error: Dynamic href \`/object/[slug]\` found in while using the \`/app\` router, this is not supported. Read more: https://nextjs.org/docs/messages/app-dir-dynamic-href"` + ) + }) } else { - it('should not error in prod', async () => { - const browser = await webdriver(next.url, '/') + it('should not error on /object in prod', async () => { + const browser = await webdriver(next.url, '/object') + expect(await browser.elementByCss('#link').text()).toBe('to slug') + }) + it('should not error on /string in prod', async () => { + const browser = await webdriver(next.url, '/string') expect(await browser.elementByCss('#link').text()).toBe('to slug') }) } diff --git a/test/e2e/app-dir/dynamic-href/app/[slug]/page.js b/test/e2e/app-dir/dynamic-href/app/object/[slug]/page.js similarity index 100% rename from test/e2e/app-dir/dynamic-href/app/[slug]/page.js rename to test/e2e/app-dir/dynamic-href/app/object/[slug]/page.js diff --git a/test/e2e/app-dir/dynamic-href/app/page.js b/test/e2e/app-dir/dynamic-href/app/object/page.js similarity index 84% rename from test/e2e/app-dir/dynamic-href/app/page.js rename to test/e2e/app-dir/dynamic-href/app/object/page.js index a428f9920eb7..bcc1956d7e73 100644 --- a/test/e2e/app-dir/dynamic-href/app/page.js +++ b/test/e2e/app-dir/dynamic-href/app/object/page.js @@ -5,7 +5,7 @@ export default function HomePage() { diff --git a/test/e2e/app-dir/dynamic-href/app/string/page.js b/test/e2e/app-dir/dynamic-href/app/string/page.js new file mode 100644 index 000000000000..a3349d59e24e --- /dev/null +++ b/test/e2e/app-dir/dynamic-href/app/string/page.js @@ -0,0 +1,9 @@ +import Link from 'next/link' + +export default function HomePage() { + return ( + + to slug + + ) +} From 8f317a4727c571100b2cd71828201e861faac94a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Tue, 22 Nov 2022 15:54:30 +0100 Subject: [PATCH 14/15] Don't error if as prop is provided --- packages/next/client/link.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index ee370ddd5766..6da55598e464 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -398,7 +398,7 @@ const Link = React.forwardRef( const isAppRouter = !pagesRouter if (process.env.NODE_ENV !== 'production') { - if (isAppRouter) { + if (isAppRouter && !asProp) { let href: string | undefined if (typeof hrefProp === 'string') { href = hrefProp From c7cca3656fe153d0c92266618d94100fbf503280 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Fri, 25 Nov 2022 13:50:44 +0100 Subject: [PATCH 15/15] Update errors/app-dir-dynamic-href.md Co-authored-by: Tim Neutkens --- errors/app-dir-dynamic-href.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/errors/app-dir-dynamic-href.md b/errors/app-dir-dynamic-href.md index ec94ed04c6da..88c76c5319c7 100644 --- a/errors/app-dir-dynamic-href.md +++ b/errors/app-dir-dynamic-href.md @@ -2,7 +2,7 @@ #### Why This Error Occurred -You have tried to use a dynamic `href` with `next/link` in the `/app` directory. This is not supported. +You have tried to use a dynamic `href` with `next/link` in the `app` directory. This is not supported as the new client-side router no longer uses a mapping of dynamic routes -> url, instead it always leverages the url. #### Possible Ways to Fix It