Skip to content

Commit

Permalink
Add error when href interpolation fails (#16946)
Browse files Browse the repository at this point in the history
This adds an error when interpolation fails to make sure invalid `href`s aren't accidentally used and an invalid URL is built. 

Closes: #16944
  • Loading branch information
ijjk committed Sep 10, 2020
1 parent c12afa0 commit 47d983f
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 16 deletions.
54 changes: 54 additions & 0 deletions errors/href-interpolation-failed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# `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 function BlogLink() {
return (
<Link
href={{
pathname: '/blog/[post]/[comment]',
query: { post: 'post-1' },
}}
>
<a>Invalid link</a>
</Link>
)
}
```

**Valid `href` interpolation**

```jsx
import Link from 'next/link'

export default function BlogLink() {
return (
<Link
href={{
pathname: '/blog/[post]/[comment]',
query: { post: 'post-1', comment: 'comment-1' },
}}
>
<a>Valid link</a>
</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)
49 changes: 35 additions & 14 deletions packages/next/next-server/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -653,32 +658,48 @@ 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]
)

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\``
)
}

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`
(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 (route === asPathname) {
const { result, params } = interpolateAs(route, asPathname, query)
} else if (shouldInterpolate) {
as = formatWithValidation(
Object.assign({}, parsedAs, {
pathname: result,
query: omitParmsFromQuery(query, params),
pathname: interpolatedAs.result,
query: omitParmsFromQuery(query, interpolatedAs.params!),
})
)
} else {
Expand Down
4 changes: 2 additions & 2 deletions test/integration/build-output/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,13 @@ 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.2).toBeLessThanOrEqual(0)
Expand Down
11 changes: 11 additions & 0 deletions test/integration/dynamic-routing/pages/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,17 @@ const Page = () => {
<a id="view-post-1-interpolated">View post 1 (interpolated)</a>
</Link>
<br />
<Link
href={{
pathname: '/[name]',
query: { another: 'value' },
}}
>
<a id="view-post-1-interpolated-incorrectly">
View post 1 (interpolated incorrectly)
</a>
</Link>
<br />
<Link
href={{
pathname: '/[name]',
Expand Down
14 changes: 14 additions & 0 deletions test/integration/dynamic-routing/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {
nextStart,
normalizeRegEx,
check,
hasRedbox,
getRedboxHeader,
} from 'next-test-utils'
import cheerio from 'cheerio'
import escapeRegex from 'escape-string-regexp'
Expand Down Expand Up @@ -788,6 +790,18 @@ function runTests(dev) {
expect(text).toBe('slug: first')
})

it('should show error when interpolating fails for href', async () => {
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(
'The provided `href` (/[name]?another=value) value is missing query values (name) to be interpolated properly.'
)
})

it('should work with HMR correctly', async () => {
const browser = await webdriver(appPort, '/post-1/comments')
let text = await browser.eval(`document.documentElement.innerHTML`)
Expand Down

0 comments on commit 47d983f

Please sign in to comment.