Skip to content

Remove extras from old params in named routes #910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 18, 2016
Merged

Remove extras from old params in named routes #910

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ab0a779
:bug: Remove extra old params in named routes
znck Nov 17, 2016
3730e88
:white_check_mark: Add tests
znck Nov 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion src/create-matcher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ const regexpCache: {
}
} = Object.create(null)

const regexpParamsCache: {
[key: string]: Array<string>
} = Object.create(null)

const regexpCompileCache: {
[key: string]: Function
} = Object.create(null)
Expand All @@ -31,14 +35,15 @@ export function createMatcher (routes: Array<RouteConfig>): Matcher {

if (name) {
const record = nameMap[name]
const paramNames = getParams(record.path)

if (typeof location.params !== 'object') {
location.params = {}
}

if (currentRoute && typeof currentRoute.params === 'object') {
for (const key in currentRoute.params) {
if (!(key in location.params)) {
if (!(key in location.params) && paramNames.indexOf(key) > -1) {
location.params[key] = currentRoute.params[key]
}
}
Expand Down Expand Up @@ -198,6 +203,26 @@ function fillParams (
}
}

function getParams (path: string): Array<string> {
const hit = regexpParamsCache[path]

if (hit) {
return hit
}

const keys = []
const paramNames: Array<string> = []

Regexp(path, keys)
Copy link
Member

@fnlctrl fnlctrl Nov 17, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


return (regexpParamsCache[path] = paramNames)
Copy link
Member

@fnlctrl fnlctrl Nov 17, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can then be simplified to

return regexpParamsCache[path] || 
   (regexpParamsCache[path] = keys.map(k => k.name))

like https://github.com/znck/vue-router/blob/ab0a779ba6f09f5b28138c9445ce84d7263daef6/src/create-matcher.js#L196-L198

}

function resolveRecordPath (path: string, record: RouteRecord): string {
return resolvePath(path, record.parent ? record.parent.path : '/', true)
}