From 2893c2c0a2ab8ff35a78c916eb7347c911db6439 Mon Sep 17 00:00:00 2001 From: Dzmitry Shylovich Date: Tue, 6 Dec 2016 21:41:01 +0300 Subject: [PATCH] fix(router): validate nested routes (#13224) Fixes #12827 --- modules/@angular/router/src/config.ts | 55 ++++++++++++++------- modules/@angular/router/test/config.spec.ts | 41 ++++++++++++--- tools/public_api_guard/router/index.d.ts | 2 +- 3 files changed, 72 insertions(+), 26 deletions(-) diff --git a/modules/@angular/router/src/config.ts b/modules/@angular/router/src/config.ts index def2c22801..e4c7a78175 100644 --- a/modules/@angular/router/src/config.ts +++ b/modules/@angular/router/src/config.ts @@ -336,21 +336,23 @@ export interface Route { canLoad?: any[]; data?: Data; resolve?: ResolveData; - children?: Route[]; + children?: Routes; loadChildren?: LoadChildren; } -export function validateConfig(config: Routes): void { +export function validateConfig(config: Routes, parentPath: string = ''): void { // forEach doesn't iterate undefined values for (let i = 0; i < config.length; i++) { - validateNode(config[i]); + const route: Route = config[i]; + const fullPath: string = getFullPath(parentPath, route); + validateNode(route, fullPath); } } -function validateNode(route: Route): void { +function validateNode(route: Route, fullPath: string): void { if (!route) { throw new Error(` - Invalid route configuration: Encountered undefined route. + Invalid configuration of route '${fullPath}': Encountered undefined route. The reason might be an extra comma. Example: @@ -362,52 +364,69 @@ function validateNode(route: Route): void { `); } if (Array.isArray(route)) { - throw new Error(`Invalid route configuration: Array cannot be specified`); + throw new Error(`Invalid configuration of route '${fullPath}': Array cannot be specified`); } if (!route.component && (route.outlet && route.outlet !== PRIMARY_OUTLET)) { throw new Error( - `Invalid route configuration of route '${route.path}': a componentless route cannot have a named outlet set`); + `Invalid configuration of route '${fullPath}': a componentless route cannot have a named outlet set`); } if (route.redirectTo && route.children) { throw new Error( - `Invalid configuration of route '${route.path}': redirectTo and children cannot be used together`); + `Invalid configuration of route '${fullPath}': redirectTo and children cannot be used together`); } if (route.redirectTo && route.loadChildren) { throw new Error( - `Invalid configuration of route '${route.path}': redirectTo and loadChildren cannot be used together`); + `Invalid configuration of route '${fullPath}': redirectTo and loadChildren cannot be used together`); } if (route.children && route.loadChildren) { throw new Error( - `Invalid configuration of route '${route.path}': children and loadChildren cannot be used together`); + `Invalid configuration of route '${fullPath}': children and loadChildren cannot be used together`); } if (route.redirectTo && route.component) { throw new Error( - `Invalid configuration of route '${route.path}': redirectTo and component cannot be used together`); + `Invalid configuration of route '${fullPath}': redirectTo and component cannot be used together`); } if (route.path && route.matcher) { throw new Error( - `Invalid configuration of route '${route.path}': path and matcher cannot be used together`); + `Invalid configuration of route '${fullPath}': path and matcher cannot be used together`); } if (route.redirectTo === void 0 && !route.component && !route.children && !route.loadChildren) { throw new Error( - `Invalid configuration of route '${route.path}': one of the following must be provided (component or redirectTo or children or loadChildren)`); + `Invalid configuration of route '${fullPath}'. One of the following must be provided: component, redirectTo, children or loadChildren`); } if (route.path === void 0 && route.matcher === void 0) { throw new Error( - `Invalid route configuration: routes must have either a path or a matcher specified`); + `Invalid configuration of route '${fullPath}': routes must have either a path or a matcher specified`); } if (typeof route.path === 'string' && route.path.charAt(0) === '/') { - throw new Error( - `Invalid route configuration of route '${route.path}': path cannot start with a slash`); + throw new Error(`Invalid configuration of route '${fullPath}': path cannot start with a slash`); } if (route.path === '' && route.redirectTo !== void 0 && route.pathMatch === void 0) { const exp = `The default value of 'pathMatch' is 'prefix', but often the intent is to use 'full'.`; throw new Error( - `Invalid route configuration of route '{path: "${route.path}", redirectTo: "${route.redirectTo}"}': please provide 'pathMatch'. ${exp}`); + `Invalid configuration of route '{path: "${fullPath}", redirectTo: "${route.redirectTo}"}': please provide 'pathMatch'. ${exp}`); } if (route.pathMatch !== void 0 && route.pathMatch !== 'full' && route.pathMatch !== 'prefix') { throw new Error( - `Invalid configuration of route '${route.path}': pathMatch can only be set to 'prefix' or 'full'`); + `Invalid configuration of route '${fullPath}': pathMatch can only be set to 'prefix' or 'full'`); + } + if (route.children) { + validateConfig(route.children, fullPath); + } +} + +function getFullPath(parentPath: string, currentRoute: Route): string { + if (!currentRoute) { + return parentPath; + } + if (!parentPath && !currentRoute.path) { + return ''; + } else if (parentPath && !currentRoute.path) { + return `${parentPath}/`; + } else if (!parentPath && currentRoute.path) { + return currentRoute.path; + } else { + return `${parentPath}/${currentRoute.path}`; } } diff --git a/modules/@angular/router/test/config.spec.ts b/modules/@angular/router/test/config.spec.ts index 20f8eb8eaf..5897da2539 100644 --- a/modules/@angular/router/test/config.spec.ts +++ b/modules/@angular/router/test/config.spec.ts @@ -25,7 +25,19 @@ describe('config', () => { it('should throw for undefined route', () => { expect(() => { validateConfig([{path: 'a', component: ComponentA}, , {path: 'b', component: ComponentB}]); - }).toThrowError(); + }).toThrowError(/Invalid configuration of route ''/); + }); + + it('should throw for undefined route in children', () => { + expect(() => { + validateConfig([{ + path: 'a', + children: [ + {path: 'b', component: ComponentB}, + , + ] + }]); + }).toThrowError(/Invalid configuration of route 'a'/); }); it('should throw when Array is passed', () => { @@ -34,7 +46,7 @@ describe('config', () => { {path: 'a', component: ComponentA}, [{path: 'b', component: ComponentB}, {path: 'c', component: ComponentC}] ]); - }).toThrowError(`Invalid route configuration: Array cannot be specified`); + }).toThrowError(`Invalid configuration of route '': Array cannot be specified`); }); it('should throw when redirectTo and children are used together', () => { @@ -46,6 +58,21 @@ describe('config', () => { `Invalid configuration of route 'a': redirectTo and children cannot be used together`); }); + it('should validate children and report full path', () => { + expect(() => validateConfig([{path: 'a', children: [{path: 'b'}]}])) + .toThrowError( + `Invalid configuration of route 'a/b'. One of the following must be provided: component, redirectTo, children or loadChildren`); + }); + + it('should properly report deeply nested path', () => { + expect(() => validateConfig([{ + path: 'a', + children: [{path: 'b', children: [{path: 'c', children: [{path: 'd'}]}]}] + }])) + .toThrowError( + `Invalid configuration of route 'a/b/c/d'. One of the following must be provided: component, redirectTo, children or loadChildren`); + }); + it('should throw when redirectTo and loadChildren are used together', () => { expect(() => { validateConfig([{path: 'a', redirectTo: 'b', loadChildren: 'value'}]); }) .toThrowError( @@ -73,26 +100,26 @@ describe('config', () => { it('should throw when path and matcher are missing', () => { expect(() => { validateConfig([{component: null, redirectTo: 'b'}]); }) .toThrowError( - `Invalid route configuration: routes must have either a path or a matcher specified`); + `Invalid configuration of route '': routes must have either a path or a matcher specified`); }); it('should throw when none of component and children or direct are missing', () => { expect(() => { validateConfig([{path: 'a'}]); }) .toThrowError( - `Invalid configuration of route 'a': one of the following must be provided (component or redirectTo or children or loadChildren)`); + `Invalid configuration of route 'a'. One of the following must be provided: component, redirectTo, children or loadChildren`); }); it('should throw when path starts with a slash', () => { expect(() => { validateConfig([{path: '/a', redirectTo: 'b'}]); - }).toThrowError(`Invalid route configuration of route '/a': path cannot start with a slash`); + }).toThrowError(`Invalid configuration of route '/a': path cannot start with a slash`); }); it('should throw when emptyPath is used with redirectTo without explicitly providing matching', () => { expect(() => { validateConfig([{path: '', redirectTo: 'b'}]); - }).toThrowError(/Invalid route configuration of route '{path: "", redirectTo: "b"}'/); + }).toThrowError(/Invalid configuration of route '{path: "", redirectTo: "b"}'/); }); it('should throw when pathPatch is invalid', () => { @@ -104,7 +131,7 @@ describe('config', () => { it('should throw when pathPatch is invalid', () => { expect(() => { validateConfig([{path: 'a', outlet: 'aux', children: []}]); }) .toThrowError( - /Invalid route configuration of route 'a': a componentless route cannot have a named outlet set/); + /Invalid configuration of route 'a': a componentless route cannot have a named outlet set/); expect(() => validateConfig([{path: 'a', outlet: '', children: []}])).not.toThrow(); expect(() => validateConfig([{path: 'a', outlet: PRIMARY_OUTLET, children: []}])) diff --git a/tools/public_api_guard/router/index.d.ts b/tools/public_api_guard/router/index.d.ts index 77cd741d30..2c3ec19ea4 100644 --- a/tools/public_api_guard/router/index.d.ts +++ b/tools/public_api_guard/router/index.d.ts @@ -186,7 +186,7 @@ export interface Route { canActivateChild?: any[]; canDeactivate?: any[]; canLoad?: any[]; - children?: Route[]; + children?: Routes; component?: Type; data?: Data; loadChildren?: LoadChildren;