Skip to content

Commit

Permalink
feat(alias): warn against redundant aliases
Browse files Browse the repository at this point in the history 
Fix #2461, #2462
  • Loading branch information
@posva
posva committed Jul 9, 2019
1 parent 799ceca commit 04a02c0
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 45 deletions.
92 changes: 57 additions & 35 deletions src/create-route-map.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ export function createRouteMap (
oldPathMap?: Dictionary<RouteRecord>,
oldNameMap?: Dictionary<RouteRecord>
): {
pathList: Array<string>;
pathMap: Dictionary<RouteRecord>;
nameMap: Dictionary<RouteRecord>;
pathList: Array<string>,
pathMap: Dictionary<RouteRecord>,
nameMap: Dictionary<RouteRecord>
} {
// the path list is used to control path matching priority
const pathList: Array<string> = oldPathList || []
Expand Down Expand Up @@ -54,17 +54,15 @@ function addRouteRecord (
assert(path != null, `"path" is required in a route configuration.`)
assert(
typeof route.component !== 'string',
`route config "component" for path: ${String(path || name)} cannot be a ` +
`string id. Use an actual component instead.`
`route config "component" for path: ${String(
path || name
)} cannot be a ` + `string id. Use an actual component instead.`
)
}

const pathToRegexpOptions: PathToRegexpOptions = route.pathToRegexpOptions || {}
const normalizedPath = normalizePath(
path,
parent,
pathToRegexpOptions.strict
)
const pathToRegexpOptions: PathToRegexpOptions =
route.pathToRegexpOptions || {}
const normalizedPath = normalizePath(path, parent, pathToRegexpOptions.strict)

if (typeof route.caseSensitive === 'boolean') {
pathToRegexpOptions.sensitive = route.caseSensitive
Expand All @@ -81,26 +79,33 @@ function addRouteRecord (
redirect: route.redirect,
beforeEnter: route.beforeEnter,
meta: route.meta || {},
props: route.props == null
? {}
: route.components
? route.props
: { default: route.props }
props:
route.props == null
? {}
: route.components
? route.props
: { default: route.props }
}

if (route.children) {
// Warn if route is named, does not redirect and has a default child route.
// If users navigate to this route by name, the default child will
// not be rendered (GH Issue #629)
if (process.env.NODE_ENV !== 'production') {
if (route.name && !route.redirect && route.children.some(child => /^\/?$/.test(child.path))) {
if (
route.name &&
!route.redirect &&
route.children.some(child => /^\/?$/.test(child.path))
) {
warn(
false,
`Named Route '${route.name}' has a default child route. ` +
`When navigating to this named route (:to="{name: '${route.name}'"), ` +
`the default child route will not be rendered. Remove the name from ` +
`this route and use the name of the default child route for named ` +
`links instead.`
`When navigating to this named route (:to="{name: '${
route.name
}'"), ` +
`the default child route will not be rendered. Remove the name from ` +
`this route and use the name of the default child route for named ` +
`links instead.`
)
}
}
Expand All @@ -112,12 +117,24 @@ function addRouteRecord (
})
}

if (!pathMap[record.path]) {
pathList.push(record.path)
pathMap[record.path] = record
}

if (route.alias !== undefined) {
const aliases = Array.isArray(route.alias)
? route.alias
: [route.alias]
const aliases = Array.isArray(route.alias) ? route.alias : [route.alias]
for (let i = 0; i < aliases.length; ++i) {
const alias = aliases[i]
if (process.env.NODE_ENV !== 'production' && alias === path) {
warn(
false,
`Found an alias with the same value as the path: "${path}". You have to remove that alias. It will be ignored in development.`
)
// skip in dev to make it work
continue
}

aliases.forEach(alias => {
const aliasRoute = {
path: alias,
children: route.children
Expand All @@ -130,12 +147,7 @@ function addRouteRecord (
parent,
record.path || '/' // matchAs
)
})
}

if (!pathMap[record.path]) {
pathList.push(record.path)
pathMap[record.path] = record
}
}

if (name) {
Expand All @@ -145,25 +157,35 @@ function addRouteRecord (
warn(
false,
`Duplicate named routes definition: ` +
`{ name: "${name}", path: "${record.path}" }`
`{ name: "${name}", path: "${record.path}" }`
)
}
}
}

function compileRouteRegex (path: string, pathToRegexpOptions: PathToRegexpOptions): RouteRegExp {
function compileRouteRegex (
path: string,
pathToRegexpOptions: PathToRegexpOptions
): RouteRegExp {
const regex = Regexp(path, [], pathToRegexpOptions)
if (process.env.NODE_ENV !== 'production') {
const keys: any = Object.create(null)
regex.keys.forEach(key => {
warn(!keys[key.name], `Duplicate param keys in route with path: "${path}"`)
warn(
!keys[key.name],
`Duplicate param keys in route with path: "${path}"`
)
keys[key.name] = true
})
}
return regex
}

function normalizePath (path: string, parent?: RouteRecord, strict?: boolean): string {
function normalizePath (
path: string,
parent?: RouteRecord,
strict?: boolean
): string {
if (!strict) path = path.replace(/\/$/, '')
if (path[0] === '/') return path
if (parent == null) return path
Expand Down
71 changes: 61 additions & 10 deletions test/unit/specs/create-map.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,28 @@ describe('Creating Route Map', function () {
})

it('has a pathList which places wildcards at the end', () => {
expect(maps.pathList).toEqual(['', '/foo', '/bar/', '/bar', '/bar-redirect/', '/bar-redirect', '*'])
expect(maps.pathList).toEqual([
'',
'/foo',
'/bar/',
'/bar',
'/bar-redirect/',
'/bar-redirect',
'*'
])
})

it('has a nameMap object for default subroute at \'bar.baz\'', function () {
it("has a nameMap object for default subroute at 'bar.baz'", function () {
expect(maps.nameMap['bar.baz']).not.toBeUndefined()
})

it('in development, has logged a warning concerning named route of parent and default subroute', function () {
process.env.NODE_ENV = 'development'
maps = createRouteMap(routes)
expect(console.warn).toHaveBeenCalledTimes(1)
expect(console.warn.calls.argsFor(0)[0]).toMatch('vue-router] Named Route \'bar\'')
expect(console.warn.calls.argsFor(0)[0]).toMatch(
"vue-router] Named Route 'bar'"
)
})

it('in development, throws if path is missing', function () {
Expand All @@ -87,22 +97,63 @@ describe('Creating Route Map', function () {
process.env.NODE_ENV = 'development'
maps = createRouteMap([
{
path: '/foo/:id', component: Foo,
children: [
{ path: 'bar/:id', component: Bar }
]
path: '/foo/:id',
component: Foo,
children: [{ path: 'bar/:id', component: Bar }]
}
])
expect(console.warn).toHaveBeenCalled()
expect(console.warn.calls.argsFor(0)[0]).toMatch(
'vue-router] Duplicate param keys in route with path: "/foo/:id/bar/:id"'
)
})

it('in development, warns about alias and path having the same value', () => {
process.env.NODE_ENV = 'development'
maps = createRouteMap([
{
path: '/foo-alias',
component: Foo,
alias: '/foo-alias'
}
])
expect(console.warn).toHaveBeenCalled()
expect(console.warn.calls.argsFor(0)[0]).toMatch(
'vue-router] Found an alias with the same value as the path: "/foo-alias"'
)
})

it('in development, warns about one alias (in an array) having the same value as the path', () => {
process.env.NODE_ENV = 'development'
maps = createRouteMap([
{
path: '/foo-alias',
component: Foo,
alias: ['/bar', '/foo-alias']
}
])
expect(console.warn).toHaveBeenCalled()
expect(console.warn.calls.argsFor(0)[0]).toMatch('vue-router] Duplicate param keys in route with path: "/foo/:id/bar/:id"')
expect(console.warn.calls.argsFor(0)[0]).toMatch(
'vue-router] Found an alias with the same value as the path: "/foo-alias"'
)
})

describe('path-to-regexp options', function () {
const routes = [
{ path: '/foo', name: 'foo', component: Foo },
{ path: '/bar', name: 'bar', component: Bar, caseSensitive: false },
{ path: '/FooBar', name: 'FooBar', component: FooBar, caseSensitive: true },
{ path: '/foobar', name: 'foobar', component: Foobar, caseSensitive: true }
{
path: '/FooBar',
name: 'FooBar',
component: FooBar,
caseSensitive: true
},
{
path: '/foobar',
name: 'foobar',
component: Foobar,
caseSensitive: true
}
]

it('caseSensitive option in route', function () {
Expand Down

0 comments on commit 04a02c0

Please sign in to comment.