From 3a307c27945b052420b16936388e299e7977088e Mon Sep 17 00:00:00 2001 From: vsavkin Date: Thu, 4 Aug 2016 18:56:22 -0700 Subject: [PATCH] fix(router): absolute redirects should work with lazy loading --- .../@angular/router/src/apply_redirects.ts | 369 ++++++++++-------- modules/@angular/router/src/recognize.ts | 16 +- .../router/test/apply_redirects.spec.ts | 15 + .../router/test/create_url_tree.spec.ts | 2 +- .../@angular/router/test/recognize.spec.ts | 27 -- 5 files changed, 218 insertions(+), 211 deletions(-) diff --git a/modules/@angular/router/src/apply_redirects.ts b/modules/@angular/router/src/apply_redirects.ts index d0283e6a7e..557630e72b 100644 --- a/modules/@angular/router/src/apply_redirects.ts +++ b/modules/@angular/router/src/apply_redirects.ts @@ -51,183 +51,210 @@ function canLoadFails(route: Route): Observable { export function applyRedirects( injector: Injector, configLoader: RouterConfigLoader, urlTree: UrlTree, config: Routes): Observable { - return expandSegmentGroup(injector, configLoader, config, urlTree.root, PRIMARY_OUTLET) - .map(rootSegmentGroup => createUrlTree(urlTree, rootSegmentGroup)) - .catch(e => { - if (e instanceof AbsoluteRedirect) { - return of (createUrlTree( - urlTree, - new UrlSegmentGroup([], {[PRIMARY_OUTLET]: new UrlSegmentGroup(e.segments, {})}))); - } else if (e instanceof NoMatch) { - throw new Error(`Cannot match any routes: '${e.segmentGroup}'`); + return new ApplyRedirects(injector, configLoader, urlTree, config).apply(); +} + +class ApplyRedirects { + private allowRedirects: boolean = true; + + constructor( + private injector: Injector, private configLoader: RouterConfigLoader, + private urlTree: UrlTree, private config: Routes) {} + + apply(): Observable { + return this.expandSegmentGroup(this.injector, this.config, this.urlTree.root, PRIMARY_OUTLET) + .map(rootSegmentGroup => this.createUrlTree(rootSegmentGroup)) + .catch(e => { + if (e instanceof AbsoluteRedirect) { + // after an absolute redirect we do not apply any more redirects! + this.allowRedirects = false; + const group = + new UrlSegmentGroup([], {[PRIMARY_OUTLET]: new UrlSegmentGroup(e.segments, {})}); + // we need to run matching, so we can fetch all lazy-loaded modules + return this.match(group); + } else if (e instanceof NoMatch) { + throw this.noMatchError(e); + } else { + throw e; + } + }); + } + + private match(segmentGroup: UrlSegmentGroup): Observable { + return this.expandSegmentGroup(this.injector, this.config, segmentGroup, PRIMARY_OUTLET) + .map(rootSegmentGroup => this.createUrlTree(rootSegmentGroup)) + .catch((e): Observable => { + if (e instanceof NoMatch) { + throw this.noMatchError(e); + } else { + throw e; + } + }); + } + + private noMatchError(e: NoMatch): any { + return new Error(`Cannot match any routes: '${e.segmentGroup}'`); + } + + private createUrlTree(rootCandidate: UrlSegmentGroup): UrlTree { + const root = rootCandidate.segments.length > 0 ? + new UrlSegmentGroup([], {[PRIMARY_OUTLET]: rootCandidate}) : + rootCandidate; + return new UrlTree(root, this.urlTree.queryParams, this.urlTree.fragment); + } + + private expandSegmentGroup( + injector: Injector, routes: Route[], segmentGroup: UrlSegmentGroup, + outlet: string): Observable { + if (segmentGroup.segments.length === 0 && segmentGroup.hasChildren()) { + return this.expandChildren(injector, routes, segmentGroup) + .map(children => new UrlSegmentGroup([], children)); + } else { + return this.expandSegment( + injector, segmentGroup, routes, segmentGroup.segments, outlet, true); + } + } + + private expandChildren(injector: Injector, routes: Route[], segmentGroup: UrlSegmentGroup): + Observable<{[name: string]: UrlSegmentGroup}> { + return waitForMap( + segmentGroup.children, + (childOutlet, child) => this.expandSegmentGroup(injector, routes, child, childOutlet)); + } + + private expandSegment( + injector: Injector, segmentGroup: UrlSegmentGroup, routes: Route[], segments: UrlSegment[], + outlet: string, allowRedirects: boolean): Observable { + const processRoutes = + of (...routes) + .map(r => { + return this + .expandSegmentAgainstRoute( + injector, segmentGroup, routes, r, segments, outlet, allowRedirects) + .catch((e) => { + if (e instanceof NoMatch) + return of (null); + else + throw e; + }); + }) + .concatAll(); + + return processRoutes.first(s => !!s).catch((e: any, _: any): Observable => { + if (e instanceof EmptyError) { + throw new NoMatch(segmentGroup); + } else { + throw e; + } + }); + } + + private expandSegmentAgainstRoute( + injector: Injector, segmentGroup: UrlSegmentGroup, routes: Route[], route: Route, + paths: UrlSegment[], outlet: string, allowRedirects: boolean): Observable { + if (getOutlet(route) !== outlet) return noMatch(segmentGroup); + if (route.redirectTo !== undefined && !(allowRedirects && this.allowRedirects)) + return noMatch(segmentGroup); + + if (route.redirectTo === undefined) { + return this.matchSegmentAgainstRoute(injector, segmentGroup, route, paths); + } else { + return this.expandSegmentAgainstRouteUsingRedirect( + injector, segmentGroup, routes, route, paths, outlet); + } + } + + private expandSegmentAgainstRouteUsingRedirect( + injector: Injector, segmentGroup: UrlSegmentGroup, routes: Route[], route: Route, + segments: UrlSegment[], outlet: string): Observable { + if (route.path === '**') { + return this.expandWildCardWithParamsAgainstRouteUsingRedirect(route); + } else { + return this.expandRegularSegmentAgainstRouteUsingRedirect( + injector, segmentGroup, routes, route, segments, outlet); + } + } + + private expandWildCardWithParamsAgainstRouteUsingRedirect(route: Route): + Observable { + const newSegments = applyRedirectCommands([], route.redirectTo, {}); + if (route.redirectTo.startsWith('/')) { + return absoluteRedirect(newSegments); + } else { + return of (new UrlSegmentGroup(newSegments, {})); + } + } + + private expandRegularSegmentAgainstRouteUsingRedirect( + injector: Injector, segmentGroup: UrlSegmentGroup, routes: Route[], route: Route, + segments: UrlSegment[], outlet: string): Observable { + const {matched, consumedSegments, lastChild, positionalParamSegments} = + match(segmentGroup, route, segments); + if (!matched) return noMatch(segmentGroup); + + const newSegments = + applyRedirectCommands(consumedSegments, route.redirectTo, positionalParamSegments); + if (route.redirectTo.startsWith('/')) { + return absoluteRedirect(newSegments); + } else { + return this.expandSegment( + injector, segmentGroup, routes, newSegments.concat(segments.slice(lastChild)), outlet, + false); + } + } + + private matchSegmentAgainstRoute( + injector: Injector, rawSegmentGroup: UrlSegmentGroup, route: Route, + segments: UrlSegment[]): Observable { + if (route.path === '**') { + return of (new UrlSegmentGroup(segments, {})); + + } else { + const {matched, consumedSegments, lastChild} = match(rawSegmentGroup, route, segments); + if (!matched) return noMatch(rawSegmentGroup); + + const rawSlicedSegments = segments.slice(lastChild); + + return this.getChildConfig(injector, route).mergeMap(routerConfig => { + const childInjector = routerConfig.injector; + const childConfig = routerConfig.routes; + const {segmentGroup, slicedSegments} = + split(rawSegmentGroup, consumedSegments, rawSlicedSegments, childConfig); + + if (slicedSegments.length === 0 && segmentGroup.hasChildren()) { + return this.expandChildren(childInjector, childConfig, segmentGroup) + .map(children => new UrlSegmentGroup(consumedSegments, children)); + + } else if (childConfig.length === 0 && slicedSegments.length === 0) { + return of (new UrlSegmentGroup(consumedSegments, {})); + } else { - throw e; + return this + .expandSegment( + childInjector, segmentGroup, childConfig, slicedSegments, PRIMARY_OUTLET, true) + .map(cs => new UrlSegmentGroup(consumedSegments.concat(cs.segments), cs.children)); } }); -} - -function createUrlTree(urlTree: UrlTree, rootCandidate: UrlSegmentGroup): UrlTree { - const root = rootCandidate.segments.length > 0 ? - new UrlSegmentGroup([], {[PRIMARY_OUTLET]: rootCandidate}) : - rootCandidate; - return new UrlTree(root, urlTree.queryParams, urlTree.fragment); -} - -function expandSegmentGroup( - injector: Injector, configLoader: RouterConfigLoader, routes: Route[], - segmentGroup: UrlSegmentGroup, outlet: string): Observable { - if (segmentGroup.segments.length === 0 && segmentGroup.hasChildren()) { - return expandChildren(injector, configLoader, routes, segmentGroup) - .map(children => new UrlSegmentGroup([], children)); - } else { - return expandSegment( - injector, configLoader, segmentGroup, routes, segmentGroup.segments, outlet, true); - } -} - -function expandChildren( - injector: Injector, configLoader: RouterConfigLoader, routes: Route[], - segmentGroup: UrlSegmentGroup): Observable<{[name: string]: UrlSegmentGroup}> { - return waitForMap( - segmentGroup.children, (childOutlet, child) => expandSegmentGroup( - injector, configLoader, routes, child, childOutlet)); -} - -function expandSegment( - injector: Injector, configLoader: RouterConfigLoader, segmentGroup: UrlSegmentGroup, - routes: Route[], segments: UrlSegment[], outlet: string, - allowRedirects: boolean): Observable { - const processRoutes = of (...routes) - .map(r => { - return expandSegmentAgainstRoute( - injector, configLoader, segmentGroup, routes, r, segments, - outlet, allowRedirects) - .catch((e) => { - if (e instanceof NoMatch) - return of (null); - else - throw e; - }); - }) - .concatAll(); - - return processRoutes.first(s => !!s).catch((e: any, _: any): Observable => { - if (e instanceof EmptyError) { - throw new NoMatch(segmentGroup); - } else { - throw e; } - }); -} - -function expandSegmentAgainstRoute( - injector: Injector, configLoader: RouterConfigLoader, segmentGroup: UrlSegmentGroup, - routes: Route[], route: Route, paths: UrlSegment[], outlet: string, - allowRedirects: boolean): Observable { - if (getOutlet(route) !== outlet) return noMatch(segmentGroup); - if (route.redirectTo !== undefined && !allowRedirects) return noMatch(segmentGroup); - - if (route.redirectTo !== undefined) { - return expandSegmentAgainstRouteUsingRedirect( - injector, configLoader, segmentGroup, routes, route, paths, outlet); - } else { - return matchSegmentAgainstRoute(injector, configLoader, segmentGroup, route, paths); } -} -function expandSegmentAgainstRouteUsingRedirect( - injector: Injector, configLoader: RouterConfigLoader, segmentGroup: UrlSegmentGroup, - routes: Route[], route: Route, segments: UrlSegment[], - outlet: string): Observable { - if (route.path === '**') { - return expandWildCardWithParamsAgainstRouteUsingRedirect(route); - } else { - return expandRegularSegmentAgainstRouteUsingRedirect( - injector, configLoader, segmentGroup, routes, route, segments, outlet); - } -} - -function expandWildCardWithParamsAgainstRouteUsingRedirect(route: Route): - Observable { - const newSegments = applyRedirectCommands([], route.redirectTo, {}); - if (route.redirectTo.startsWith('/')) { - return absoluteRedirect(newSegments); - } else { - return of (new UrlSegmentGroup(newSegments, {})); - } -} - -function expandRegularSegmentAgainstRouteUsingRedirect( - injector: Injector, configLoader: RouterConfigLoader, segmentGroup: UrlSegmentGroup, - routes: Route[], route: Route, segments: UrlSegment[], - outlet: string): Observable { - const {matched, consumedSegments, lastChild, positionalParamSegments} = - match(segmentGroup, route, segments); - if (!matched) return noMatch(segmentGroup); - - const newSegments = - applyRedirectCommands(consumedSegments, route.redirectTo, positionalParamSegments); - if (route.redirectTo.startsWith('/')) { - return absoluteRedirect(newSegments); - } else { - return expandSegment( - injector, configLoader, segmentGroup, routes, newSegments.concat(segments.slice(lastChild)), - outlet, false); - } -} - -function matchSegmentAgainstRoute( - injector: Injector, configLoader: RouterConfigLoader, rawSegmentGroup: UrlSegmentGroup, - route: Route, segments: UrlSegment[]): Observable { - if (route.path === '**') { - return of (new UrlSegmentGroup(segments, {})); - - } else { - const {matched, consumedSegments, lastChild} = match(rawSegmentGroup, route, segments); - if (!matched) return noMatch(rawSegmentGroup); - - const rawSlicedSegments = segments.slice(lastChild); - - return getChildConfig(injector, configLoader, route).mergeMap(routerConfig => { - const childInjector = routerConfig.injector; - const childConfig = routerConfig.routes; - const {segmentGroup, slicedSegments} = - split(rawSegmentGroup, consumedSegments, rawSlicedSegments, childConfig); - - if (slicedSegments.length === 0 && segmentGroup.hasChildren()) { - return expandChildren(childInjector, configLoader, childConfig, segmentGroup) - .map(children => new UrlSegmentGroup(consumedSegments, children)); - - } else if (childConfig.length === 0 && slicedSegments.length === 0) { - return of (new UrlSegmentGroup(consumedSegments, {})); - - } else { - return expandSegment( - childInjector, configLoader, segmentGroup, childConfig, slicedSegments, - PRIMARY_OUTLET, true) - .map(cs => new UrlSegmentGroup(consumedSegments.concat(cs.segments), cs.children)); - } - }); - } -} - -function getChildConfig(injector: Injector, configLoader: RouterConfigLoader, route: Route): - Observable { - if (route.children) { - return of (new LoadedRouterConfig(route.children, injector, null)); - } else if (route.loadChildren) { - return runGuards(injector, route).mergeMap(shouldLoad => { - if (shouldLoad) { - return configLoader.load(injector, route.loadChildren).map(r => { - (route)._loadedConfig = r; - return r; - }); - } else { - return canLoadFails(route); - } - }); - } else { - return of (new LoadedRouterConfig([], injector, null)); + private getChildConfig(injector: Injector, route: Route): Observable { + if (route.children) { + return of (new LoadedRouterConfig(route.children, injector, null)); + } else if (route.loadChildren) { + return runGuards(injector, route).mergeMap(shouldLoad => { + if (shouldLoad) { + return this.configLoader.load(injector, route.loadChildren).map(r => { + (route)._loadedConfig = r; + return r; + }); + } else { + return canLoadFails(route); + } + }); + } else { + return of (new LoadedRouterConfig([], injector, null)); + } } } diff --git a/modules/@angular/router/src/recognize.ts b/modules/@angular/router/src/recognize.ts index 3e76405467..7b08f3c484 100644 --- a/modules/@angular/router/src/recognize.ts +++ b/modules/@angular/router/src/recognize.ts @@ -18,9 +18,7 @@ import {UrlSegment, UrlSegmentGroup, UrlTree, mapChildrenIntoArray} from './url_ import {last, merge} from './utils/collection'; import {TreeNode} from './utils/tree'; -class NoMatch { - constructor(public segmentGroup: UrlSegmentGroup = null) {} -} +class NoMatch {} class InheritedFromParent { constructor( @@ -65,14 +63,8 @@ class Recognizer { return of (new RouterStateSnapshot(this.url, rootNode)); } catch (e) { - if (e instanceof NoMatch) { - return new Observable( - (obs: Observer) => - obs.error(new Error(`Cannot match any routes: '${e.segmentGroup}'`))); - } else { - return new Observable( - (obs: Observer) => obs.error(e)); - } + return new Observable( + (obs: Observer) => obs.error(e)); } } @@ -108,7 +100,7 @@ class Recognizer { if (!(e instanceof NoMatch)) throw e; } } - throw new NoMatch(segmentGroup); + throw new NoMatch(); } processSegmentAgainstRoute( diff --git a/modules/@angular/router/test/apply_redirects.spec.ts b/modules/@angular/router/test/apply_redirects.spec.ts index bc986f10f6..6ee3938b96 100644 --- a/modules/@angular/router/test/apply_redirects.spec.ts +++ b/modules/@angular/router/test/apply_redirects.spec.ts @@ -251,6 +251,21 @@ describe('applyRedirects', () => { (r) => { compareTrees(r, tree('/a/b')); }, (e) => { throw 'Should not reach'; }); }); + + it('should work with absolute redirects', () => { + const loadedConfig = new LoadedRouterConfig( + [{path: '', component: ComponentB}], 'stubInjector', 'stubFactoryResolver'); + + const loader = {load: (injector: any, p: any) => of (loadedConfig)}; + + const config = + [{path: '', pathMatch: 'full', redirectTo: '/a'}, {path: 'a', loadChildren: 'children'}]; + + applyRedirects('providedInjector', loader, tree(''), config).forEach(r => { + compareTrees(r, tree('a')); + expect((config[1])._loadedConfig).toBe(loadedConfig); + }); + }); }); describe('empty paths', () => { diff --git a/modules/@angular/router/test/create_url_tree.spec.ts b/modules/@angular/router/test/create_url_tree.spec.ts index 21d33d91d8..3a589e4e80 100644 --- a/modules/@angular/router/test/create_url_tree.spec.ts +++ b/modules/@angular/router/test/create_url_tree.spec.ts @@ -13,7 +13,7 @@ import {ActivatedRoute, ActivatedRouteSnapshot, advanceActivatedRoute} from '../ import {PRIMARY_OUTLET, Params} from '../src/shared'; import {DefaultUrlSerializer, UrlSegment, UrlSegmentGroup, UrlTree} from '../src/url_tree'; -fdescribe('createUrlTree', () => { +describe('createUrlTree', () => { const serializer = new DefaultUrlSerializer(); it('should navigate to the root', () => { diff --git a/modules/@angular/router/test/recognize.spec.ts b/modules/@angular/router/test/recognize.spec.ts index c011eb84e3..7e1cc12250 100644 --- a/modules/@angular/router/test/recognize.spec.ts +++ b/modules/@angular/router/test/recognize.spec.ts @@ -259,19 +259,6 @@ describe('recognize', () => { }); }); - it('should not match when terminal', () => { - recognize( - RootComponent, [{ - path: '', - pathMatch: 'full', - component: ComponentA, - children: [{path: 'b', component: ComponentB}] - }], - tree('b'), '') - .subscribe( - () => {}, (e) => { expect(e.message).toEqual('Cannot match any routes: \'b\''); }); - }); - it('should work (nested case)', () => { checkRecognize( [{path: '', component: ComponentA, children: [{path: '', component: ComponentB}]}], '', @@ -678,20 +665,6 @@ describe('recognize', () => { 'Two segments cannot have the same outlet name: \'aux:b\' and \'aux:c\'.'); }); }); - - it('should error when no matching routes', () => { - recognize(RootComponent, [{path: 'a', component: ComponentA}], tree('invalid'), 'invalid') - .subscribe((_) => {}, (s: RouterStateSnapshot) => { - expect(s.toString()).toContain('Cannot match any routes'); - }); - }); - - it('should error when no matching routes (too short)', () => { - recognize(RootComponent, [{path: 'a/:id', component: ComponentA}], tree('a'), 'a') - .subscribe((_) => {}, (s: RouterStateSnapshot) => { - expect(s.toString()).toContain('Cannot match any routes'); - }); - }); }); });