fix(resolve): exclude empty optional arguments from params
fix https://github.com/vuejs/router/issues/2419
Removed if optional parameter is empty Now the param will match if :to is string and if it is object
In the case of string path, it should not be an empty string parameter(Right?) , so I thought there would be no problem with excluding it.
However, this may be a breaking change if you have users who were using this
In addition, I added a router-link to Playgound to check the operation.
Deploy Preview for vue-router canceled.
| Name | Link |
|---|---|
| Latest commit | 66f824f7f9a2b8f71828267b32ec6a14ce60d7e4 |
| Latest deploy log | https://app.netlify.com/sites/vue-router/deploys/677e37044abd7e000807f3dd |
I was wondering about some other edge cases. I'm not sure whether these necessarily impact this PR, but I thought they were worth considered before it's merged.
I put together this Playground to demonstrate:
These are are to relevant values:
'/features'{ name: 'features' }{ name: 'features', params: { id: undefined } }{ name: 'features', params: { id: null } }{ name: 'features', params: { id: '' } }
All of them lead to the same path.
Clicking around between them, it seems that 2, 3 and 4 are considered equivalent, which makes sense. It's 5 that I found most interesting. It seems to behave like it's a child of 2, 3 and 4.
I also tried with { name: 'features', params: { id: '123' } }, which seems to behave similarly. When that route is active it shows 2, 3 and 4 as active, though not exact active.
Not sure if that's intentional or not, but it doesn't seem to match what's described at https://router.vuejs.org/guide/essentials/active-links.html. I wrote that page, so I may be to blame for describing these classes incorrectly. It seems that there's some special handling for optional params that I didn't take into account in my description.
FYI in the upcoming custom resolvers I'm trying to introduce a consistent behaviour. Right now I'm going for this: optional params are always null if missing (not passed, empty string, undefined, null). It's working well and the only issue I can see is that the empty string could also be simply rejected rather than transformed to null but it sounded less convenient. Open to suggestions!