From 04a02c0b2082475434ad7355809a21f7e5ecc6a0 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Tue, 9 Jul 2019 21:08:45 +0200 Subject: [PATCH] feat(alias): warn against redundant aliases Fix #2461, #2462 --- src/create-route-map.js | 92 ++++++++++++++++++------------ test/unit/specs/create-map.spec.js | 71 +++++++++++++++++++---- 2 files changed, 118 insertions(+), 45 deletions(-) diff --git a/src/create-route-map.js b/src/create-route-map.js index 560e05fdd..0f790108c 100644 --- a/src/create-route-map.js +++ b/src/create-route-map.js @@ -10,9 +10,9 @@ export function createRouteMap ( oldPathMap?: Dictionary, oldNameMap?: Dictionary ): { - pathList: Array; - pathMap: Dictionary; - nameMap: Dictionary; + pathList: Array, + pathMap: Dictionary, + nameMap: Dictionary } { // the path list is used to control path matching priority const pathList: Array = oldPathList || [] @@ -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 @@ -81,11 +79,12 @@ 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) { @@ -93,14 +92,20 @@ function addRouteRecord ( // 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.` ) } } @@ -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 @@ -130,12 +147,7 @@ function addRouteRecord ( parent, record.path || '/' // matchAs ) - }) - } - - if (!pathMap[record.path]) { - pathList.push(record.path) - pathMap[record.path] = record + } } if (name) { @@ -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 diff --git a/test/unit/specs/create-map.spec.js b/test/unit/specs/create-map.spec.js index 67fba131a..a7d09daff 100644 --- a/test/unit/specs/create-map.spec.js +++ b/test/unit/specs/create-map.spec.js @@ -57,10 +57,18 @@ 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() }) @@ -68,7 +76,9 @@ describe('Creating Route Map', 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 () { @@ -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 () {