Skip to content

Commit

Permalink
fix(router): validate nested routes (#13224)
Browse files Browse the repository at this point in the history 
Fixes #12827
  • Loading branch information
@alxhub
Dzmitry Shylovich authored and alxhub committed Dec 6, 2016
1 parent 393c100 commit 2893c2c
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 26 deletions.
55 changes: 37 additions & 18 deletions modules/@angular/router/src/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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}`;
}
}
41 changes: 34 additions & 7 deletions modules/@angular/router/test/config.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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(
Expand Down Expand Up @@ -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([<any>{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([<any>{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', () => {
Expand All @@ -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: []}]))
Expand Down
2 changes: 1 addition & 1 deletion tools/public_api_guard/router/index.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export interface Route {
canActivateChild?: any[];
canDeactivate?: any[];
canLoad?: any[];
children?: Route[];
children?: Routes;
component?: Type<any>;
data?: Data;
loadChildren?: LoadChildren;
Expand Down

0 comments on commit 2893c2c

Please sign in to comment.