Remove extras from old params in named routes #910
Conversation
znck
changed the title
|
Thanks for the PR! I'll review it asap. |
There was a problem hiding this comment.
@znck Also, can we add accompanying e2e tests for it (that addresses #906)?
The test provided in the original PR (https://github.com/vuejs/vue-router/pull/851/files) didn't actually cover the case of using extra params, we should probably fix it too so that this PR can cover both cases (bugfix and using extra params)
| const keys = [] | ||
| const paramNames: Array<string> = [] | ||
|
|
||
| Regexp(path, keys) |
There was a problem hiding this comment.
We don't have to call Regexp(path) here again (without cache, actually), there's already code for getting cached keys and regexp here (https://github.com/znck/vue-router/blob/ab0a779ba6f09f5b28138c9445ce84d7263daef6/src/create-matcher.js#L163-L172).
We should probably create a separate function that uses the code above, take a path as only parameter, returns a {keys, regexp} object, maybe name it getKeysAndParams.
Then we can get keys with ES6 destructuring
const {keys} = getKeysAndParams(path)| for (let i = 0, len = keys.length; i < len; ++i) { | ||
| const key = keys[i] | ||
| if (key && 'name' in key) paramNames.push(key.name) | ||
| } |
There was a problem hiding this comment.
This whole for-loop can be simplified to keys.map(k => k.name) for brevity. Since we are already using cache here, the performance penalty of using a Array.prototype.map is negligible.
| if (key && 'name' in key) paramNames.push(key.name) | ||
| } | ||
|
|
||
| return (regexpParamsCache[path] = paramNames) |
There was a problem hiding this comment.
This can then be simplified to
return regexpParamsCache[path] ||
(regexpParamsCache[path] = keys.map(k => k.name))
|
+1 |
Fixes #906