From cee67e6fe040d33274fb65e1823f7ead9251fb78 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Mon, 23 Nov 2015 16:34:40 -0800 Subject: [PATCH] Revert "feat(router): allow linking to auxiliary routes" This reverts commit 0b1ff2db9ea0a2faf1cbff778520794fe9a48d0b. --- .../angular2/src/router/route_recognizer.ts | 16 --- modules/angular2/src/router/route_registry.ts | 102 ++++++++++-------- .../router/integration/router_link_spec.ts | 32 +----- .../test/router/route_registry_spec.ts | 24 ++--- 4 files changed, 64 insertions(+), 110 deletions(-) diff --git a/modules/angular2/src/router/route_recognizer.ts b/modules/angular2/src/router/route_recognizer.ts index 6511fadda7..7c7dd8a69a 100644 --- a/modules/angular2/src/router/route_recognizer.ts +++ b/modules/angular2/src/router/route_recognizer.ts @@ -26,10 +26,6 @@ import {ComponentInstruction} from './instruction'; export class RouteRecognizer { names = new Map(); - // map from name to recognizer - auxNames = new Map(); - - // map from starting path to recognizer auxRoutes = new Map(); // TODO: optimize this into a trie @@ -52,12 +48,8 @@ export class RouteRecognizer { let path = config.path.startsWith('/') ? config.path.substring(1) : config.path; var recognizer = new PathRecognizer(config.path, handler); this.auxRoutes.set(path, recognizer); - if (isPresent(config.name)) { - this.auxNames.set(config.name, recognizer); - } return recognizer.terminal; } - if (config instanceof Redirect) { this.redirects.push(new Redirector(config.path, config.redirectTo)); return true; @@ -135,14 +127,6 @@ export class RouteRecognizer { } return pathRecognizer.generate(params); } - - generateAuxiliary(name: string, params: any): ComponentInstruction { - var pathRecognizer: PathRecognizer = this.auxNames.get(name); - if (isBlank(pathRecognizer)) { - return null; - } - return pathRecognizer.generate(params); - } } export class Redirector { diff --git a/modules/angular2/src/router/route_registry.ts b/modules/angular2/src/router/route_registry.ts index c132d363ab..7dba2eea81 100644 --- a/modules/angular2/src/router/route_registry.ts +++ b/modules/angular2/src/router/route_registry.ts @@ -5,7 +5,6 @@ import {ListWrapper, Map, MapWrapper, StringMapWrapper} from 'angular2/src/facad import {Promise, PromiseWrapper} from 'angular2/src/facade/async'; import { isPresent, - isArray, isBlank, isType, isString, @@ -189,61 +188,70 @@ export class RouteRegistry { * Given a normalized list with component names and params like: `['user', {id: 3 }]` * generates a url with a leading slash relative to the provided `parentComponent`. */ - generate(linkParams: any[], parentComponent: any, _aux = false): Instruction { - let linkIndex = 0; - let routeName = linkParams[linkIndex]; + generate(linkParams: any[], parentComponent: any): Instruction { + let segments = []; + let componentCursor = parentComponent; + var lastInstructionIsTerminal = false; - // TODO: this is kind of odd but it makes existing assertions pass - if (isBlank(parentComponent)) { - throw new BaseException(`Could not find route named "${routeName}".`); + for (let i = 0; i < linkParams.length; i += 1) { + let segment = linkParams[i]; + if (isBlank(componentCursor)) { + throw new BaseException(`Could not find route named "${segment}".`); + } + if (!isString(segment)) { + throw new BaseException(`Unexpected segment "${segment}" in link DSL. Expected a string.`); + } else if (segment == '' || segment == '.' || segment == '..') { + throw new BaseException(`"${segment}/" is only allowed at the beginning of a link DSL.`); + } + let params = {}; + if (i + 1 < linkParams.length) { + let nextSegment = linkParams[i + 1]; + if (isStringMap(nextSegment)) { + params = nextSegment; + i += 1; + } + } + + var componentRecognizer = this._rules.get(componentCursor); + if (isBlank(componentRecognizer)) { + throw new BaseException( + `Component "${getTypeNameForDebugging(componentCursor)}" has no route config.`); + } + var response = componentRecognizer.generate(segment, params); + + if (isBlank(response)) { + throw new BaseException( + `Component "${getTypeNameForDebugging(componentCursor)}" has no route named "${segment}".`); + } + segments.push(response); + componentCursor = response.componentType; + lastInstructionIsTerminal = response.terminal; } - if (!isString(routeName)) { - throw new BaseException(`Unexpected segment "${routeName}" in link DSL. Expected a string.`); - } else if (routeName == '' || routeName == '.' || routeName == '..') { - throw new BaseException(`"${routeName}/" is only allowed at the beginning of a link DSL.`); - } + var instruction: Instruction = null; - let params = {}; - if (linkIndex + 1 < linkParams.length) { - let nextSegment = linkParams[linkIndex + 1]; - if (isStringMap(nextSegment) && !isArray(nextSegment)) { - params = nextSegment; - linkIndex += 1; + if (!lastInstructionIsTerminal) { + instruction = this._generateRedirects(componentCursor); + + if (isPresent(instruction)) { + let lastInstruction = instruction; + while (isPresent(lastInstruction.child)) { + lastInstruction = lastInstruction.child; + } + lastInstructionIsTerminal = lastInstruction.component.terminal; + } + if (isPresent(componentCursor) && !lastInstructionIsTerminal) { + throw new BaseException( + `Link "${ListWrapper.toJSON(linkParams)}" does not resolve to a terminal or async instruction.`); } } - let auxInstructions: {[key: string]: Instruction} = {}; - var nextSegment; - while (linkIndex + 1 < linkParams.length && isArray(nextSegment = linkParams[linkIndex + 1])) { - auxInstructions[nextSegment[0]] = this.generate(nextSegment, parentComponent, true); - linkIndex += 1; + + while (segments.length > 0) { + instruction = new Instruction(segments.pop(), instruction, {}); } - var componentRecognizer = this._rules.get(parentComponent); - if (isBlank(componentRecognizer)) { - throw new BaseException( - `Component "${getTypeNameForDebugging(parentComponent)}" has no route config.`); - } - - var componentInstruction = _aux ? componentRecognizer.generateAuxiliary(routeName, params) : - componentRecognizer.generate(routeName, params); - - if (isBlank(componentInstruction)) { - throw new BaseException( - `Component "${getTypeNameForDebugging(parentComponent)}" has no route named "${routeName}".`); - } - - var childInstruction = null; - if (linkIndex + 1 < linkParams.length) { - var remaining = linkParams.slice(linkIndex + 1); - childInstruction = this.generate(remaining, componentInstruction.componentType); - } else if (!componentInstruction.terminal) { - throw new BaseException( - `Link "${ListWrapper.toJSON(linkParams)}" does not resolve to a terminal or async instruction.`); - } - - return new Instruction(componentInstruction, childInstruction, auxInstructions); + return instruction; } public hasRoute(name: string, parentComponent: any): boolean { diff --git a/modules/angular2/test/router/integration/router_link_spec.ts b/modules/angular2/test/router/integration/router_link_spec.ts index 1d67ab694e..079840eb24 100644 --- a/modules/angular2/test/router/integration/router_link_spec.ts +++ b/modules/angular2/test/router/integration/router_link_spec.ts @@ -31,7 +31,6 @@ import { RouterLink, RouterOutlet, AsyncRoute, - AuxRoute, Route, RouteParams, RouteConfig, @@ -199,7 +198,7 @@ export function main() { name: 'ChildWithGrandchild' }) ])) - .then((_) => router.navigateByUrl('/child-with-grandchild/grandchild')) + .then((_) => router.navigate(['/ChildWithGrandchild'])) .then((_) => { fixture.detectChanges(); expect(DOM.getAttribute(fixture.debugElement.componentViewChildren[1] @@ -235,21 +234,6 @@ export function main() { }); })); - it('should generate links to auxiliary routes', inject([AsyncTestCompleter], (async) => { - compile() - .then((_) => router.config([new Route({path: '/...', component: AuxLinkCmp})])) - .then((_) => router.navigateByUrl('/')) - .then((_) => { - rootTC.detectChanges(); - expect(DOM.getAttribute(rootTC.debugElement.componentViewChildren[1] - .componentViewChildren[0] - .nativeElement, - 'href')) - .toEqual('/(aside)'); - async.done(); - }); - })); - describe('router-link-active CSS class', () => { it('should be added to the associated element', inject([AsyncTestCompleter], (async) => { @@ -487,17 +471,3 @@ class AmbiguousBookCmp { title: string; constructor(params: RouteParams) { this.title = params.get('title'); } } - -@Component({selector: 'aux-cmp'}) -@View({ - template: - `aside | - | aside `, - directives: ROUTER_DIRECTIVES -}) -@RouteConfig([ - new Route({path: '/', component: HelloCmp, name: 'Hello'}), - new AuxRoute({path: '/aside', component: Hello2Cmp, name: 'Aside'}) -]) -class AuxLinkCmp { -} diff --git a/modules/angular2/test/router/route_registry_spec.ts b/modules/angular2/test/router/route_registry_spec.ts index 3b906bb749..5487dd61ed 100644 --- a/modules/angular2/test/router/route_registry_spec.ts +++ b/modules/angular2/test/router/route_registry_spec.ts @@ -51,7 +51,7 @@ export function main() { .toEqual('second'); }); - xit('should generate URLs that account for redirects', () => { + it('should generate URLs that account for redirects', () => { registry.config( RootHostCmp, new Route({path: '/first/...', component: DummyParentRedirectCmp, name: 'FirstCmp'})); @@ -60,7 +60,7 @@ export function main() { .toEqual('first/second'); }); - xit('should generate URLs in a hierarchy of redirects', () => { + it('should generate URLs in a hierarchy of redirects', () => { registry.config( RootHostCmp, new Route({path: '/first/...', component: DummyMultipleRedirectCmp, name: 'FirstCmp'})); @@ -89,7 +89,7 @@ export function main() { inject([AsyncTestCompleter], (async) => { registry.config( RootHostCmp, - new AsyncRoute({path: '/first/...', loader: asyncParentLoader, name: 'FirstCmp'})); + new AsyncRoute({path: '/first/...', loader: AsyncParentLoader, name: 'FirstCmp'})); expect(() => registry.generate(['FirstCmp', 'SecondCmp'], RootHostCmp)) .toThrowError('Could not find route named "SecondCmp".'); @@ -103,20 +103,12 @@ export function main() { }); })); + it('should throw when generating a url and a parent has no config', () => { expect(() => registry.generate(['FirstCmp', 'SecondCmp'], RootHostCmp)) .toThrowError('Component "RootHostCmp" has no route config.'); }); - it('should generate URLs for aux routes', () => { - registry.config(RootHostCmp, - new Route({path: '/primary', component: DummyCmpA, name: 'Primary'})); - registry.config(RootHostCmp, new AuxRoute({path: '/aux', component: DummyCmpB, name: 'Aux'})); - - expect(stringifyInstruction(registry.generate(['Primary', ['Aux']], RootHostCmp))) - .toEqual('primary(aux)'); - }); - it('should prefer static segments to dynamic', inject([AsyncTestCompleter], (async) => { registry.config(RootHostCmp, new Route({path: '/:site', component: DummyCmpB})); registry.config(RootHostCmp, new Route({path: '/home', component: DummyCmpA})); @@ -201,7 +193,7 @@ export function main() { it('should match the URL using an async parent component', inject([AsyncTestCompleter], (async) => { registry.config(RootHostCmp, - new AsyncRoute({path: '/first/...', loader: asyncParentLoader})); + new AsyncRoute({path: '/first/...', loader: AsyncParentLoader})); registry.recognize('/first/second', RootHostCmp) .then((instruction) => { @@ -283,17 +275,17 @@ export function main() { }); } -function asyncParentLoader() { +function AsyncParentLoader() { return PromiseWrapper.resolve(DummyParentCmp); } -function asyncChildLoader() { +function AsyncChildLoader() { return PromiseWrapper.resolve(DummyCmpB); } class RootHostCmp {} -@RouteConfig([new AsyncRoute({path: '/second', loader: asyncChildLoader})]) +@RouteConfig([new AsyncRoute({path: '/second', loader: AsyncChildLoader})]) class DummyAsyncCmp { }