vuejs / router

🚦 The official router for Vue.js
https://router.vuejs.org/
MIT License
3.88k stars 1.18k forks source link

feat(dx): warn when passing undefined/null locations #2158

Closed skirtles-code closed 7 months ago

skirtles-code commented 7 months ago

This PR attempts to improve the warnings and error handling for a common problem encountered during development.

Background

There's an error message I've encountered several times on Vue Land:

TypeError: Cannot use 'in' operator to search for 'path' in undefined

This comes from router.resolve(), e.g.:

That's a silly example, it's rarely quite so obvious when someone is having this problem in a real application.

These two issues are both hitting the same problem (the first one is in the wrong repo):

You get something similar with null instead of undefined.

router.push(undefined) gives the same error, as that uses router.resolve().

Most commonly this is hit via useLink(). Here's a simple example of that:

Again, I've made it obvious what the problem is with to: undefined, but real code is rarely so blatant.

It is possible to hit this same error via RouterLink, but in that case we get a prop validation warning, making it a bit easier to figure out what the problem is:

The mistake in that example is a typo in patn: '/about'.

But even with the prop validation warning, it can be difficult to identify exactly which component has the problem. If we could inspect the components using Vue Devtools it'd be much easier, but that isn't currently an option because rendering bombs out.

The Vuetify problem

Someone on Vue Land was hitting this error, but without any warnings to indicate what they were doing wrong. They weren't using RouterLink, they were using Vuetify's breadcrumb component.

The ultimate cause of the problem was an undefined in their data that they hadn't taken into account. But this isn't quite as simple as it sounds.

There's this problematic line in Vuetify:

The problem here is that props.to can be undefined. While the Vuetify composable does check that, it only checks the initial value. In the application we encountered on Vue Land, the to prop was populated initially, only becoming undefined after some new data loaded.

It isn't obvious to me how Vuetify could fix this problem in their own code. There doesn't seem to be a way to disable the useLink() composable when the prop is undefined and re-enable it when the value is defined.

This PR doesn't attempt to address that problem directly, it just aims to improve the warnings/errors to ease debugging.

What's changed

In cases where RouterLink has an undefined or null value for to, this will lead to 3 warnings being shown: one for prop validation, one for useLink() and one for router.resolve(). I couldn't see an easy way to avoid that, but I think that's much better than not having the warnings.

netlify[bot] commented 7 months ago

Deploy Preview for vue-router canceled.

Name Link
Latest commit ff140d1693950a572ea74c28114b3174ff04137a
Latest deploy log https://app.netlify.com/sites/vue-router/deploys/65e6a915ff44d90008ae6f74
codecov-commenter commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 91.01%. Comparing base (13303bd) to head (ff140d1). Report is 3 commits behind head on main.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2158 +/- ## ========================================== + Coverage 90.90% 91.01% +0.11% ========================================== Files 24 24 Lines 1121 1135 +14 Branches 347 351 +4 ========================================== + Hits 1019 1033 +14 Misses 63 63 Partials 39 39 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.