From 4e8911eb7188eb229dd87ca962b9b4fafdb0aac5 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 8 Sep 2020 11:46:30 -0500 Subject: [PATCH 1/4] Add err.sh --- errors/href-interpolation-failed.md | 52 +++++++++++++++++++ .../next/next-server/lib/router/router.ts | 27 +++++++--- .../dynamic-routing/pages/index.js | 11 ++++ .../dynamic-routing/test/index.test.js | 14 +++++ 4 files changed, 98 insertions(+), 6 deletions(-) create mode 100644 errors/href-interpolation-failed.md diff --git a/errors/href-interpolation-failed.md b/errors/href-interpolation-failed.md new file mode 100644 index 0000000000000..6566447cd6b30 --- /dev/null +++ b/errors/href-interpolation-failed.md @@ -0,0 +1,52 @@ +# `href` Interpolation Failed + +#### Why This Error Occurred + +Somewhere you are utilizing the `next/link` component, `Router#push`, or `Router#replace` with `href` interpolation to build dynamic routes but did not provide all of the needed dynamic route params to properly interpolate the `href`. + +Note: this error will only show when the `next/link` component is clicked not when only rendered. + +**Invalid `href` interpolation** + +```jsx +import Link from 'next/link' + +export default () => ( + <> + + Invalid link + + +) +``` + +**Valid `href` interpolation** + +```jsx +import Link from 'next/link' + +export default () => ( + <> + + Valid link + + +) +``` + +#### Possible Ways to Fix It + +Look for any usage of the `next/link` component, `Router#push`, or `Router#replace` and make sure that the provided `href` has all needed dynamic params in the query. + +### Useful Links + +- [Routing section in Documentation](https://nextjs.org/docs/routing/introduction) +- [Dynamic routing section in Documentation](https://nextjs.org/docs/routing/dynamic-routes) diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index 0192ace936736..792cfc5db97ed 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -178,11 +178,14 @@ export function resolveHref( finalUrl.pathname, query ) - interpolatedAs = formatWithValidation({ - pathname: result, - hash: finalUrl.hash, - query: omitParmsFromQuery(query, params), - }) + + if (result) { + interpolatedAs = formatWithValidation({ + pathname: result, + hash: finalUrl.hash, + query: omitParmsFromQuery(query, params), + }) + } } // if the origin didn't change, it means we received a relative href @@ -191,7 +194,9 @@ export function resolveHref( ? finalUrl.href.slice(finalUrl.origin.length) : finalUrl.href - return (resolveAs ? [resolvedHref, interpolatedAs] : resolvedHref) as string + return (resolveAs + ? [resolvedHref, interpolatedAs || resolvedHref] + : resolvedHref) as string } catch (_) { return (resolveAs ? [urlAsString] : urlAsString) as string } @@ -656,6 +661,16 @@ export default class Router implements BaseRouter { } } else if (route === asPathname) { const { result, params } = interpolateAs(route, asPathname, query) + + if (!result) { + throw new Error( + `Interpolation failed for href (${route}) due to not all param values being provided in the query, needed param values (${params.join( + ', ' + )})\n` + + `Read more: https://err.sh/next.js/href-interpolation-failed` + ) + } + as = formatWithValidation( Object.assign({}, parsedAs, { pathname: result, diff --git a/test/integration/dynamic-routing/pages/index.js b/test/integration/dynamic-routing/pages/index.js index 68a2815320ace..b7f95c48102ec 100644 --- a/test/integration/dynamic-routing/pages/index.js +++ b/test/integration/dynamic-routing/pages/index.js @@ -31,6 +31,17 @@ const Page = () => { View post 1 (interpolated)
+ + + View post 1 (interpolated incorrectly) + + +
{ + const browser = await webdriver(appPort, '/') + await browser + .elementByCss('#view-post-1-interpolated-incorrectly') + .click() + await hasRedbox(browser) + const header = await getRedboxHeader(browser) + expect(header).toContain( + `Interpolation failed for href (/[name]) due to not all param values being provided in the query, needed param values (name)` + ) + }) + it('should work with HMR correctly', async () => { const browser = await webdriver(appPort, '/post-1/comments') let text = await browser.eval(`document.documentElement.innerHTML`) From 4ffa1527f5724fdb9856f8198b9551214edd02cd Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 8 Sep 2020 12:31:14 -0500 Subject: [PATCH 2/4] Update size tests --- test/integration/build-output/test/index.test.js | 6 +++--- test/integration/size-limit/test/index.test.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/integration/build-output/test/index.test.js b/test/integration/build-output/test/index.test.js index 5acf77f1072f2..13eaa9e560ba7 100644 --- a/test/integration/build-output/test/index.test.js +++ b/test/integration/build-output/test/index.test.js @@ -95,16 +95,16 @@ describe('Build Output', () => { expect(indexSize.endsWith('B')).toBe(true) // should be no bigger than 60.2 kb - expect(parseFloat(indexFirstLoad) - 60.4).toBeLessThanOrEqual(0) + expect(parseFloat(indexFirstLoad) - 60.5).toBeLessThanOrEqual(0) expect(indexFirstLoad.endsWith('kB')).toBe(true) expect(parseFloat(err404Size) - 3.5).toBeLessThanOrEqual(0) expect(err404Size.endsWith('kB')).toBe(true) - expect(parseFloat(err404FirstLoad) - 63.6).toBeLessThanOrEqual(0) + expect(parseFloat(err404FirstLoad) - 63.7).toBeLessThanOrEqual(0) expect(err404FirstLoad.endsWith('kB')).toBe(true) - expect(parseFloat(sharedByAll) - 60.1).toBeLessThanOrEqual(0) + expect(parseFloat(sharedByAll) - 60.2).toBeLessThanOrEqual(0) expect(sharedByAll.endsWith('kB')).toBe(true) if (_appSize.endsWith('kB')) { diff --git a/test/integration/size-limit/test/index.test.js b/test/integration/size-limit/test/index.test.js index 6bc2f88f42233..69d1d60c16753 100644 --- a/test/integration/size-limit/test/index.test.js +++ b/test/integration/size-limit/test/index.test.js @@ -100,7 +100,7 @@ describe('Production response size', () => { ) // These numbers are without gzip compression! - const delta = responseSizesBytes - 169 * 1024 + const delta = responseSizesBytes - 170 * 1024 expect(delta).toBeLessThanOrEqual(1024) // don't increase size more than 1kb expect(delta).toBeGreaterThanOrEqual(-1024) // don't decrease size more than 1kb without updating target }) From e23a114040683c131094149107953efa3bc287b8 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Thu, 10 Sep 2020 10:04:51 -0400 Subject: [PATCH 3/4] Fix error desc --- errors/href-interpolation-failed.md | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/errors/href-interpolation-failed.md b/errors/href-interpolation-failed.md index 6566447cd6b30..a3526207cba52 100644 --- a/errors/href-interpolation-failed.md +++ b/errors/href-interpolation-failed.md @@ -11,8 +11,8 @@ Note: this error will only show when the `next/link` component is clicked not wh ```jsx import Link from 'next/link' -export default () => ( - <> +export default function BlogLink() { + return ( ( > Invalid link - -) + ) +} ``` **Valid `href` interpolation** @@ -30,16 +30,18 @@ export default () => ( ```jsx import Link from 'next/link' -export default () => ( - <> - +export default function BlogLink() { + return ( + Valid link - -) + ) +} ``` #### Possible Ways to Fix It From 9cb5b64683c65ac7f4a137985852e4b55c5b83d4 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 10 Sep 2020 14:29:27 -0500 Subject: [PATCH 4/4] Combine error messages --- .../next/next-server/lib/router/router.ts | 42 +++++++++++-------- .../dynamic-routing/test/index.test.js | 2 +- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index 69ad9d6178450..0ef8fde67aa97 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -658,7 +658,12 @@ export default class Router implements BaseRouter { const routeRegex = getRouteRegex(route) const routeMatch = getRouteMatcher(routeRegex)(asPathname) - if (!routeMatch) { + const shouldInterpolate = route === asPathname + const interpolatedAs = shouldInterpolate + ? interpolateAs(route, asPathname, query) + : ({} as { result: undefined; params: undefined }) + + if (!routeMatch || (shouldInterpolate && !interpolatedAs.result)) { const missingParams = Object.keys(routeRegex.groups).filter( (param) => !query[param] ) @@ -666,7 +671,11 @@ export default class Router implements BaseRouter { if (missingParams.length > 0) { if (process.env.NODE_ENV !== 'production') { console.warn( - `Mismatching \`as\` and \`href\` failed to manually provide ` + + `${ + shouldInterpolate + ? `Interpolating href` + : `Mismatching \`as\` and \`href\`` + } failed to manually provide ` + `the params: ${missingParams.join( ', ' )} in the \`href\`'s \`query\`` @@ -674,26 +683,23 @@ export default class Router implements BaseRouter { } throw new Error( - `The provided \`as\` value (${asPathname}) is incompatible with the \`href\` value (${route}). ` + - `Read more: https://err.sh/vercel/next.js/incompatible-href-as` - ) - } - } else if (route === asPathname) { - const { result, params } = interpolateAs(route, asPathname, query) - - if (!result) { - throw new Error( - `Interpolation failed for href (${route}) due to not all param values being provided in the query, needed param values (${params.join( - ', ' - )})\n` + - `Read more: https://err.sh/next.js/href-interpolation-failed` + (shouldInterpolate + ? `The provided \`href\` (${url}) value is missing query values (${missingParams.join( + ', ' + )}) to be interpolated properly. ` + : `The provided \`as\` value (${asPathname}) is incompatible with the \`href\` value (${route}). `) + + `Read more: https://err.sh/vercel/next.js/${ + shouldInterpolate + ? 'href-interpolation-failed' + : 'incompatible-href-as' + }` ) } - + } else if (shouldInterpolate) { as = formatWithValidation( Object.assign({}, parsedAs, { - pathname: result, - query: omitParmsFromQuery(query, params), + pathname: interpolatedAs.result, + query: omitParmsFromQuery(query, interpolatedAs.params!), }) ) } else { diff --git a/test/integration/dynamic-routing/test/index.test.js b/test/integration/dynamic-routing/test/index.test.js index dbf1b0ee0ef0a..f8b64e5f7a177 100644 --- a/test/integration/dynamic-routing/test/index.test.js +++ b/test/integration/dynamic-routing/test/index.test.js @@ -798,7 +798,7 @@ function runTests(dev) { await hasRedbox(browser) const header = await getRedboxHeader(browser) expect(header).toContain( - `Interpolation failed for href (/[name]) due to not all param values being provided in the query, needed param values (name)` + 'The provided `href` (/[name]?another=value) value is missing query values (name) to be interpolated properly.' ) })