Skip to content

Commit 47d983f

Browse files
authored
Add error when href interpolation fails (vercel#16946)
This adds an error when interpolation fails to make sure invalid `href`s aren't accidentally used and an invalid URL is built. Closes: vercel#16944
1 parent c12afa0 commit 47d983f

File tree

5 files changed

+116
-16
lines changed

5 files changed

+116
-16
lines changed

errors/href-interpolation-failed.md

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
# `href` Interpolation Failed
2+
3+
#### Why This Error Occurred
4+
5+
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`.
6+
7+
Note: this error will only show when the `next/link` component is clicked not when only rendered.
8+
9+
**Invalid `href` interpolation**
10+
11+
```jsx
12+
import Link from 'next/link'
13+
14+
export default function BlogLink() {
15+
return (
16+
<Link
17+
href={{
18+
pathname: '/blog/[post]/[comment]',
19+
query: { post: 'post-1' },
20+
}}
21+
>
22+
<a>Invalid link</a>
23+
</Link>
24+
)
25+
}
26+
```
27+
28+
**Valid `href` interpolation**
29+
30+
```jsx
31+
import Link from 'next/link'
32+
33+
export default function BlogLink() {
34+
return (
35+
<Link
36+
href={{
37+
pathname: '/blog/[post]/[comment]',
38+
query: { post: 'post-1', comment: 'comment-1' },
39+
}}
40+
>
41+
<a>Valid link</a>
42+
</Link>
43+
)
44+
}
45+
```
46+
47+
#### Possible Ways to Fix It
48+
49+
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.
50+
51+
### Useful Links
52+
53+
- [Routing section in Documentation](https://nextjs.org/docs/routing/introduction)
54+
- [Dynamic routing section in Documentation](https://nextjs.org/docs/routing/dynamic-routes)

packages/next/next-server/lib/router/router.ts

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -178,11 +178,14 @@ export function resolveHref(
178178
finalUrl.pathname,
179179
query
180180
)
181-
interpolatedAs = formatWithValidation({
182-
pathname: result,
183-
hash: finalUrl.hash,
184-
query: omitParmsFromQuery(query, params),
185-
})
181+
182+
if (result) {
183+
interpolatedAs = formatWithValidation({
184+
pathname: result,
185+
hash: finalUrl.hash,
186+
query: omitParmsFromQuery(query, params),
187+
})
188+
}
186189
}
187190

188191
// if the origin didn't change, it means we received a relative href
@@ -191,7 +194,9 @@ export function resolveHref(
191194
? finalUrl.href.slice(finalUrl.origin.length)
192195
: finalUrl.href
193196

194-
return (resolveAs ? [resolvedHref, interpolatedAs] : resolvedHref) as string
197+
return (resolveAs
198+
? [resolvedHref, interpolatedAs || resolvedHref]
199+
: resolvedHref) as string
195200
} catch (_) {
196201
return (resolveAs ? [urlAsString] : urlAsString) as string
197202
}
@@ -653,32 +658,48 @@ export default class Router implements BaseRouter {
653658

654659
const routeRegex = getRouteRegex(route)
655660
const routeMatch = getRouteMatcher(routeRegex)(asPathname)
656-
if (!routeMatch) {
661+
const shouldInterpolate = route === asPathname
662+
const interpolatedAs = shouldInterpolate
663+
? interpolateAs(route, asPathname, query)
664+
: ({} as { result: undefined; params: undefined })
665+
666+
if (!routeMatch || (shouldInterpolate && !interpolatedAs.result)) {
657667
const missingParams = Object.keys(routeRegex.groups).filter(
658668
(param) => !query[param]
659669
)
660670

661671
if (missingParams.length > 0) {
662672
if (process.env.NODE_ENV !== 'production') {
663673
console.warn(
664-
`Mismatching \`as\` and \`href\` failed to manually provide ` +
674+
`${
675+
shouldInterpolate
676+
? `Interpolating href`
677+
: `Mismatching \`as\` and \`href\``
678+
} failed to manually provide ` +
665679
`the params: ${missingParams.join(
666680
', '
667681
)} in the \`href\`'s \`query\``
668682
)
669683
}
670684

671685
throw new Error(
672-
`The provided \`as\` value (${asPathname}) is incompatible with the \`href\` value (${route}). ` +
673-
`Read more: https://err.sh/vercel/next.js/incompatible-href-as`
686+
(shouldInterpolate
687+
? `The provided \`href\` (${url}) value is missing query values (${missingParams.join(
688+
', '
689+
)}) to be interpolated properly. `
690+
: `The provided \`as\` value (${asPathname}) is incompatible with the \`href\` value (${route}). `) +
691+
`Read more: https://err.sh/vercel/next.js/${
692+
shouldInterpolate
693+
? 'href-interpolation-failed'
694+
: 'incompatible-href-as'
695+
}`
674696
)
675697
}
676-
} else if (route === asPathname) {
677-
const { result, params } = interpolateAs(route, asPathname, query)
698+
} else if (shouldInterpolate) {
678699
as = formatWithValidation(
679700
Object.assign({}, parsedAs, {
680-
pathname: result,
681-
query: omitParmsFromQuery(query, params),
701+
pathname: interpolatedAs.result,
702+
query: omitParmsFromQuery(query, interpolatedAs.params!),
682703
})
683704
)
684705
} else {

test/integration/build-output/test/index.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,13 @@ describe('Build Output', () => {
9595
expect(indexSize.endsWith('B')).toBe(true)
9696

9797
// should be no bigger than 60.2 kb
98-
expect(parseFloat(indexFirstLoad) - 60.4).toBeLessThanOrEqual(0)
98+
expect(parseFloat(indexFirstLoad) - 60.5).toBeLessThanOrEqual(0)
9999
expect(indexFirstLoad.endsWith('kB')).toBe(true)
100100

101101
expect(parseFloat(err404Size) - 3.5).toBeLessThanOrEqual(0)
102102
expect(err404Size.endsWith('kB')).toBe(true)
103103

104-
expect(parseFloat(err404FirstLoad) - 63.6).toBeLessThanOrEqual(0)
104+
expect(parseFloat(err404FirstLoad) - 63.7).toBeLessThanOrEqual(0)
105105
expect(err404FirstLoad.endsWith('kB')).toBe(true)
106106

107107
expect(parseFloat(sharedByAll) - 60.2).toBeLessThanOrEqual(0)

test/integration/dynamic-routing/pages/index.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,17 @@ const Page = () => {
3131
<a id="view-post-1-interpolated">View post 1 (interpolated)</a>
3232
</Link>
3333
<br />
34+
<Link
35+
href={{
36+
pathname: '/[name]',
37+
query: { another: 'value' },
38+
}}
39+
>
40+
<a id="view-post-1-interpolated-incorrectly">
41+
View post 1 (interpolated incorrectly)
42+
</a>
43+
</Link>
44+
<br />
3445
<Link
3546
href={{
3647
pathname: '/[name]',

test/integration/dynamic-routing/test/index.test.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import {
1515
nextStart,
1616
normalizeRegEx,
1717
check,
18+
hasRedbox,
19+
getRedboxHeader,
1820
} from 'next-test-utils'
1921
import cheerio from 'cheerio'
2022
import escapeRegex from 'escape-string-regexp'
@@ -788,6 +790,18 @@ function runTests(dev) {
788790
expect(text).toBe('slug: first')
789791
})
790792

793+
it('should show error when interpolating fails for href', async () => {
794+
const browser = await webdriver(appPort, '/')
795+
await browser
796+
.elementByCss('#view-post-1-interpolated-incorrectly')
797+
.click()
798+
await hasRedbox(browser)
799+
const header = await getRedboxHeader(browser)
800+
expect(header).toContain(
801+
'The provided `href` (/[name]?another=value) value is missing query values (name) to be interpolated properly.'
802+
)
803+
})
804+
791805
it('should work with HMR correctly', async () => {
792806
const browser = await webdriver(appPort, '/post-1/comments')
793807
let text = await browser.eval(`document.documentElement.innerHTML`)

0 commit comments

Comments
 (0)