From 6ccbfd41dd1bcc8c2b7e2a86f893d91104856b9a Mon Sep 17 00:00:00 2001 From: vsavkin Date: Tue, 25 Oct 2016 14:33:18 -0700 Subject: [PATCH] fix(router): preserve resolve data Closes #12306 --- modules/@angular/router/src/recognize.ts | 95 +++++++------------ modules/@angular/router/src/router.ts | 11 ++- modules/@angular/router/src/router_state.ts | 62 ++++++++---- .../@angular/router/test/integration.spec.ts | 30 +++++- .../@angular/router/test/recognize.spec.ts | 21 +--- modules/@angular/router/test/router.spec.ts | 15 +-- 6 files changed, 119 insertions(+), 115 deletions(-) diff --git a/modules/@angular/router/src/recognize.ts b/modules/@angular/router/src/recognize.ts index 837834ec9f..6e3c7a01de 100644 --- a/modules/@angular/router/src/recognize.ts +++ b/modules/@angular/router/src/recognize.ts @@ -12,7 +12,7 @@ import {Observer} from 'rxjs/Observer'; import {of } from 'rxjs/observable/of'; import {Data, ResolveData, Route, Routes} from './config'; -import {ActivatedRouteSnapshot, InheritedResolve, RouterStateSnapshot} from './router_state'; +import {ActivatedRouteSnapshot, RouterStateSnapshot, inheritedParamsDataResolve} from './router_state'; import {PRIMARY_OUTLET, Params} from './shared'; import {UrlSegment, UrlSegmentGroup, UrlTree, mapChildrenIntoArray} from './url_tree'; import {last, merge} from './utils/collection'; @@ -20,22 +20,6 @@ import {TreeNode} from './utils/tree'; class NoMatch {} -class InheritedFromParent { - constructor( - public parent: InheritedFromParent, public snapshot: ActivatedRouteSnapshot, - public params: Params, public data: Data, public resolve: InheritedResolve) {} - - get allParams(): Params { - return this.parent ? merge(this.parent.allParams, this.params) : this.params; - } - - get allData(): Data { return this.parent ? merge(this.parent.allData, this.data) : this.data; } - - static empty(snapshot: ActivatedRouteSnapshot): InheritedFromParent { - return new InheritedFromParent(null, snapshot, {}, {}, new InheritedResolve(null, {})); - } -} - export function recognize( rootComponentType: Type, config: Routes, urlTree: UrlTree, url: string): Observable { @@ -51,17 +35,16 @@ class Recognizer { try { const rootSegmentGroup = split(this.urlTree.root, [], [], this.config).segmentGroup; - const children = this.processSegmentGroup( - this.config, rootSegmentGroup, InheritedFromParent.empty(null), PRIMARY_OUTLET); + const children = this.processSegmentGroup(this.config, rootSegmentGroup, PRIMARY_OUTLET); const root = new ActivatedRouteSnapshot( [], Object.freeze({}), Object.freeze(this.urlTree.queryParams), this.urlTree.fragment, {}, - PRIMARY_OUTLET, this.rootComponentType, null, this.urlTree.root, -1, - InheritedResolve.empty); + PRIMARY_OUTLET, this.rootComponentType, null, this.urlTree.root, -1, {}); const rootNode = new TreeNode(root, children); - - return of (new RouterStateSnapshot(this.url, rootNode)); + const routeState = new RouterStateSnapshot(this.url, rootNode); + this.inheriteParamsAndData(routeState._root); + return of (routeState); } catch (e) { return new Observable( @@ -69,22 +52,29 @@ class Recognizer { } } + inheriteParamsAndData(routeNode: TreeNode): void { + const route = routeNode.value; - processSegmentGroup( - config: Route[], segmentGroup: UrlSegmentGroup, inherited: InheritedFromParent, - outlet: string): TreeNode[] { + const i = inheritedParamsDataResolve(route); + route.params = Object.freeze(i.params); + route.data = Object.freeze(i.data); + + routeNode.children.forEach(n => this.inheriteParamsAndData(n)); + } + + processSegmentGroup(config: Route[], segmentGroup: UrlSegmentGroup, outlet: string): + TreeNode[] { if (segmentGroup.segments.length === 0 && segmentGroup.hasChildren()) { - return this.processChildren(config, segmentGroup, inherited); + return this.processChildren(config, segmentGroup); } else { - return this.processSegment(config, segmentGroup, 0, segmentGroup.segments, inherited, outlet); + return this.processSegment(config, segmentGroup, 0, segmentGroup.segments, outlet); } } - processChildren(config: Route[], segmentGroup: UrlSegmentGroup, inherited: InheritedFromParent): + processChildren(config: Route[], segmentGroup: UrlSegmentGroup): TreeNode[] { const children = mapChildrenIntoArray( - segmentGroup, - (child, childOutlet) => this.processSegmentGroup(config, child, inherited, childOutlet)); + segmentGroup, (child, childOutlet) => this.processSegmentGroup(config, child, childOutlet)); checkOutletNameUniqueness(children); sortActivatedRouteSnapshots(children); return children; @@ -92,11 +82,10 @@ class Recognizer { processSegment( config: Route[], segmentGroup: UrlSegmentGroup, pathIndex: number, segments: UrlSegment[], - inherited: InheritedFromParent, outlet: string): TreeNode[] { + outlet: string): TreeNode[] { for (let r of config) { try { - return this.processSegmentAgainstRoute( - r, segmentGroup, pathIndex, segments, inherited, outlet); + return this.processSegmentAgainstRoute(r, segmentGroup, pathIndex, segments, outlet); } catch (e) { if (!(e instanceof NoMatch)) throw e; } @@ -106,26 +95,21 @@ class Recognizer { processSegmentAgainstRoute( route: Route, rawSegment: UrlSegmentGroup, pathIndex: number, segments: UrlSegment[], - inherited: InheritedFromParent, outlet: string): TreeNode[] { + outlet: string): TreeNode[] { if (route.redirectTo) throw new NoMatch(); if ((route.outlet ? route.outlet : PRIMARY_OUTLET) !== outlet) throw new NoMatch(); - const newInheritedResolve = new InheritedResolve(inherited.resolve, getResolve(route)); - if (route.path === '**') { const params = segments.length > 0 ? last(segments).parameters : {}; const snapshot = new ActivatedRouteSnapshot( - segments, Object.freeze(merge(inherited.allParams, params)), - Object.freeze(this.urlTree.queryParams), this.urlTree.fragment, - merge(inherited.allData, getData(route)), outlet, route.component, route, - getSourceSegmentGroup(rawSegment), getPathIndexShift(rawSegment) + segments.length, - newInheritedResolve); + segments, params, Object.freeze(this.urlTree.queryParams), this.urlTree.fragment, + getData(route), outlet, route.component, route, getSourceSegmentGroup(rawSegment), + getPathIndexShift(rawSegment) + segments.length, getResolve(route)); return [new TreeNode(snapshot, [])]; } - const {consumedSegments, parameters, lastChild} = - match(rawSegment, route, segments, inherited.snapshot); + const {consumedSegments, parameters, lastChild} = match(rawSegment, route, segments); const rawSlicedSegments = segments.slice(lastChild); const childConfig = getChildConfig(route); @@ -133,19 +117,14 @@ class Recognizer { split(rawSegment, consumedSegments, rawSlicedSegments, childConfig); const snapshot = new ActivatedRouteSnapshot( - consumedSegments, Object.freeze(merge(inherited.allParams, parameters)), - Object.freeze(this.urlTree.queryParams), this.urlTree.fragment, - merge(inherited.allData, getData(route)), outlet, route.component, route, + consumedSegments, parameters, Object.freeze(this.urlTree.queryParams), + this.urlTree.fragment, getData(route), outlet, route.component, route, getSourceSegmentGroup(rawSegment), getPathIndexShift(rawSegment) + consumedSegments.length, - newInheritedResolve); + getResolve(route)); - const newInherited = route.component ? - InheritedFromParent.empty(snapshot) : - new InheritedFromParent( - inherited, snapshot, parameters, getData(route), newInheritedResolve); if (slicedSegments.length === 0 && segmentGroup.hasChildren()) { - const children = this.processChildren(childConfig, segmentGroup, newInherited); + const children = this.processChildren(childConfig, segmentGroup); return [new TreeNode(snapshot, children)]; } else if (childConfig.length === 0 && slicedSegments.length === 0) { @@ -153,8 +132,7 @@ class Recognizer { } else { const children = this.processSegment( - childConfig, segmentGroup, pathIndex + lastChild, slicedSegments, newInherited, - PRIMARY_OUTLET); + childConfig, segmentGroup, pathIndex + lastChild, slicedSegments, PRIMARY_OUTLET); return [new TreeNode(snapshot, children)]; } } @@ -178,15 +156,12 @@ function getChildConfig(route: Route): Route[] { } } -function match( - segmentGroup: UrlSegmentGroup, route: Route, segments: UrlSegment[], - parent: ActivatedRouteSnapshot) { +function match(segmentGroup: UrlSegmentGroup, route: Route, segments: UrlSegment[]) { if (route.path === '') { if (route.pathMatch === 'full' && (segmentGroup.hasChildren() || segments.length > 0)) { throw new NoMatch(); } else { - const params = parent ? parent.params : {}; - return {consumedSegments: [], lastChild: 0, parameters: params}; + return {consumedSegments: [], lastChild: 0, parameters: {}}; } } diff --git a/modules/@angular/router/src/router.ts b/modules/@angular/router/src/router.ts index 50b4b9705f..6a6bfe659e 100644 --- a/modules/@angular/router/src/router.ts +++ b/modules/@angular/router/src/router.ts @@ -21,14 +21,14 @@ import {mergeMap} from 'rxjs/operator/mergeMap'; import {reduce} from 'rxjs/operator/reduce'; import {applyRedirects} from './apply_redirects'; -import {ResolveData, Routes, validateConfig} from './config'; +import {Data, ResolveData, Routes, validateConfig} from './config'; import {createRouterState} from './create_router_state'; import {createUrlTree} from './create_url_tree'; import {RouterOutlet} from './directives/router_outlet'; import {recognize} from './recognize'; import {LoadedRouterConfig, RouterConfigLoader} from './router_config_loader'; import {RouterOutletMap} from './router_outlet_map'; -import {ActivatedRoute, ActivatedRouteSnapshot, RouterState, RouterStateSnapshot, advanceActivatedRoute, createEmptyState} from './router_state'; +import {ActivatedRoute, ActivatedRouteSnapshot, RouterState, RouterStateSnapshot, advanceActivatedRoute, createEmptyState, inheritedParamsDataResolve} from './router_state'; import {NavigationCancelingError, PRIMARY_OUTLET, Params} from './shared'; import {DefaultUrlHandlingStrategy, UrlHandlingStrategy} from './url_handling_strategy'; import {UrlSerializer, UrlTree, containsTree, createEmptyUrlTree} from './url_tree'; @@ -781,6 +781,7 @@ export class PreActivation { } else { // we need to set the data future.data = curr.data; + future._resolvedData = curr._resolvedData; } // If we have a component, we need to go through an outlet. @@ -881,9 +882,9 @@ export class PreActivation { private runResolve(future: ActivatedRouteSnapshot): Observable { const resolve = future._resolve; - return map.call(this.resolveNode(resolve.current, future), (resolvedData: any): any => { - resolve.resolvedData = resolvedData; - future.data = merge(future.data, resolve.flattenedResolvedData); + return map.call(this.resolveNode(resolve, future), (resolvedData: any): any => { + future._resolvedData = resolvedData; + future.data = merge(future.data, inheritedParamsDataResolve(future).resolve); return null; }); } diff --git a/modules/@angular/router/src/router_state.ts b/modules/@angular/router/src/router_state.ts index 9aa4e4e8bc..c704ffd3e1 100644 --- a/modules/@angular/router/src/router_state.ts +++ b/modules/@angular/router/src/router_state.ts @@ -83,7 +83,7 @@ export function createEmptyStateSnapshot( const fragment = ''; const activated = new ActivatedRouteSnapshot( [], emptyParams, emptyQueryParams, fragment, emptyData, PRIMARY_OUTLET, rootComponent, null, - urlTree.root, -1, InheritedResolve.empty); + urlTree.root, -1, {}); return new RouterStateSnapshot('', new TreeNode(activated, [])); } @@ -207,25 +207,44 @@ export class ActivatedRoute { /** * @internal */ -export class InheritedResolve { - /** - * @internal - */ - resolvedData = {}; - - constructor(public parent: InheritedResolve, public current: ResolveData) {} - - /** - * @internal - */ - get flattenedResolvedData(): Data { - return this.parent ? merge(this.parent.flattenedResolvedData, this.resolvedData) : - this.resolvedData; - } - - static get empty(): InheritedResolve { return new InheritedResolve(null, {}); } +export type Inherited = { + params: Params; data: Data; resolve: Data; } +/** + * @internal + */ +export function +inheritedParamsDataResolve(route: ActivatedRouteSnapshot): + Inherited { + const pathToRoot = route.pathFromRoot; + + let inhertingStartingFrom = pathToRoot.length - 1; + + while (inhertingStartingFrom >= 1) { + const current = pathToRoot[inhertingStartingFrom]; + const parent = pathToRoot[inhertingStartingFrom - 1]; + // current route is an empty path => inherits its parent's params and data + if (current.routeConfig && current.routeConfig.path === '') { + inhertingStartingFrom--; + + // parent is componentless => current route should inherit its params and data + } else if (!parent.component) { + inhertingStartingFrom--; + + } else { + break; + } + } + + return pathToRoot.slice(inhertingStartingFrom).reduce((res, curr) => { + const params = merge(res.params, curr.params); + const data = merge(res.data, curr.data); + const resolve = merge(res.resolve, curr._resolvedData); + return {params, data, resolve}; + }, {params: {}, data: {}, resolve: {}}); + } + /** * @whatItDoes Contains the information about a route associated with a component loaded in an * outlet @@ -258,7 +277,10 @@ export class ActivatedRouteSnapshot { _lastPathIndex: number; /** @internal */ - _resolve: InheritedResolve; + _resolve: ResolveData; + + /** @internal */ + _resolvedData: Data; /** @internal */ _routerState: RouterStateSnapshot; @@ -301,7 +323,7 @@ export class ActivatedRouteSnapshot { * The component of the route. */ public component: Type|string, routeConfig: Route, urlSegment: UrlSegmentGroup, - lastPathIndex: number, resolve: InheritedResolve) { + lastPathIndex: number, resolve: ResolveData) { this._routeConfig = routeConfig; this._urlSegment = urlSegment; this._lastPathIndex = lastPathIndex; diff --git a/modules/@angular/router/test/integration.spec.ts b/modules/@angular/router/test/integration.spec.ts index 302b347c51..158dff2dec 100644 --- a/modules/@angular/router/test/integration.spec.ts +++ b/modules/@angular/router/test/integration.spec.ts @@ -695,6 +695,30 @@ describe('Integration', () => { expect(e).toEqual('error'); }))); + + it('should preserve resolved data', + fakeAsync(inject([Router, Location], (router: Router, location: Location) => { + const fixture = createRoot(router, RootCmp); + + router.resetConfig([{ + path: 'parent', + resolve: {two: 'resolveTwo'}, + children: [ + {path: 'child1', component: CollectParamsCmp}, + {path: 'child2', component: CollectParamsCmp} + ] + }]); + + let e: any = null; + router.navigateByUrl('/parent/child1'); + advance(fixture); + + router.navigateByUrl('/parent/child2'); + advance(fixture); + + const cmp = fixture.debugElement.children[1].componentInstance; + expect(cmp.route.snapshot.data).toEqual({two: 2}); + }))); }); describe('router links', () => { @@ -2102,9 +2126,9 @@ class CollectParamsCmp { private params: any = []; private urls: any = []; - constructor(a: ActivatedRoute) { - a.params.forEach(p => this.params.push(p)); - a.url.forEach(u => this.urls.push(u)); + constructor(private route: ActivatedRoute) { + route.params.forEach(p => this.params.push(p)); + route.url.forEach(u => this.urls.push(u)); } recordedUrls(): string[] { diff --git a/modules/@angular/router/test/recognize.spec.ts b/modules/@angular/router/test/recognize.spec.ts index 1dac06b896..43408f4925 100644 --- a/modules/@angular/router/test/recognize.spec.ts +++ b/modules/@angular/router/test/recognize.spec.ts @@ -219,26 +219,7 @@ describe('recognize', () => { [{path: 'a', resolve: {one: 'some-token'}, component: ComponentA}], 'a', (s: RouterStateSnapshot) => { const r: ActivatedRouteSnapshot = s.firstChild(s.root); - expect(r._resolve.current).toEqual({one: 'some-token'}); - }); - }); - - it('should reuse componentless route\'s resolve', () => { - checkRecognize( - [{ - path: 'a', - resolve: {one: 'one'}, - children: [ - {path: '', resolve: {two: 'two'}, component: ComponentB}, - {path: '', resolve: {three: 'three'}, component: ComponentC, outlet: 'aux'} - ] - }], - 'a', (s: RouterStateSnapshot) => { - const a: ActivatedRouteSnapshot = s.firstChild(s.root); - const c: ActivatedRouteSnapshot[] = s.children(a); - - expect(c[0]._resolve.parent).toBe(a._resolve); - expect(c[1]._resolve.parent).toBe(a._resolve); + expect(r._resolve).toEqual({one: 'some-token'}); }); }); }); diff --git a/modules/@angular/router/test/router.spec.ts b/modules/@angular/router/test/router.spec.ts index 3722fd5e1d..27a959770f 100644 --- a/modules/@angular/router/test/router.spec.ts +++ b/modules/@angular/router/test/router.spec.ts @@ -8,9 +8,10 @@ import {TestBed} from '@angular/core/testing'; +import {ResolveData} from '../src/config'; import {PreActivation, Router} from '../src/router'; import {RouterOutletMap} from '../src/router_outlet_map'; -import {ActivatedRouteSnapshot, InheritedResolve, RouterStateSnapshot, createEmptyStateSnapshot} from '../src/router_state'; +import {ActivatedRouteSnapshot, RouterStateSnapshot, createEmptyStateSnapshot} from '../src/router_state'; import {DefaultUrlSerializer} from '../src/url_tree'; import {TreeNode} from '../src/utils/tree'; import {RouterTestingModule} from '../testing/router_testing_module'; @@ -39,7 +40,7 @@ describe('Router', () => { beforeEach(() => { empty = createEmptyStateSnapshot(serializer.parse('/'), null); }); it('should resolve data', () => { - const r = new InheritedResolve(InheritedResolve.empty, {data: 'resolver'}); + const r = {data: 'resolver'}; const n = createActivatedRouteSnapshot('a', {resolve: r}); const s = new RouterStateSnapshot('url', new TreeNode(empty.root, [new TreeNode(n, [])])); @@ -49,10 +50,10 @@ describe('Router', () => { }); it('should wait for the parent resolve to complete', () => { - const parentResolve = new InheritedResolve(InheritedResolve.empty, {data: 'resolver'}); - const childResolve = new InheritedResolve(parentResolve, {}); + const parentResolve = {data: 'resolver'}; + const childResolve = {}; - const parent = createActivatedRouteSnapshot('a', {resolve: parentResolve}); + const parent = createActivatedRouteSnapshot(null, {resolve: parentResolve}); const child = createActivatedRouteSnapshot('b', {resolve: childResolve}); const s = new RouterStateSnapshot( @@ -66,8 +67,8 @@ describe('Router', () => { }); it('should copy over data when creating a snapshot', () => { - const r1 = new InheritedResolve(InheritedResolve.empty, {data: 'resolver1'}); - const r2 = new InheritedResolve(InheritedResolve.empty, {data: 'resolver2'}); + const r1 = {data: 'resolver1'}; + const r2 = {data: 'resolver2'}; const n1 = createActivatedRouteSnapshot('a', {resolve: r1}); const s1 = new RouterStateSnapshot('url', new TreeNode(empty.root, [new TreeNode(n1, [])]));