Skip to content

Commit

Permalink
feat(router): add "paramsInheritanceStrategy" router configuration op…
Browse files Browse the repository at this point in the history 
…tion

Previously, the router would merge path and matrix params, as well as
data/resolve, with special rules (only merging down when the route has
an empty path, or is component-less). This change adds an extra option
"paramsInheritanceStrategy" which, when set to 'always', makes child
routes unconditionally inherit params from parent routes.

Closes #20572.
  • Loading branch information
@voithos @alxhub
voithos authored and alxhub committed Dec 20, 2017
1 parent 5f23a12 commit 5efea2f
Show file tree
Hide file tree
Showing 10 changed files with 204 additions and 48 deletions.
15 changes: 9 additions & 6 deletions packages/router/src/pre_activation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {reduce} from 'rxjs/operator/reduce';
import {LoadedRouterConfig, ResolveData, RunGuardsAndResolvers} from './config';
import {ActivationStart, ChildActivationStart, Event} from './events';
import {ChildrenOutletContexts, OutletContext} from './router_outlet_context';
import {ActivatedRouteSnapshot, RouterStateSnapshot, equalParamsAndUrlSegments, inheritedParamsDataResolve} from './router_state';
import {ActivatedRouteSnapshot, ParamsInheritanceStrategy, RouterStateSnapshot, equalParamsAndUrlSegments, inheritedParamsDataResolve} from './router_state';
import {andObservables, forEach, shallowEqual, wrapIntoObservable} from './utils/collection';
import {TreeNode, nodeChildrenAsMap} from './utils/tree';

Expand Down Expand Up @@ -63,11 +63,11 @@ export class PreActivation {
(canDeactivate: boolean) => canDeactivate ? this.runCanActivateChecks() : of (false));
}

resolveData(): Observable<any> {
resolveData(paramsInheritanceStrategy: ParamsInheritanceStrategy): Observable<any> {
if (!this.isActivating()) return of (null);
const checks$ = from(this.canActivateChecks);
const runningChecks$ =
concatMap.call(checks$, (check: CanActivate) => this.runResolve(check.route));
const runningChecks$ = concatMap.call(
checks$, (check: CanActivate) => this.runResolve(check.route, paramsInheritanceStrategy));
return reduce.call(runningChecks$, (_: any, __: any) => _);
}

Expand Down Expand Up @@ -306,11 +306,14 @@ export class PreActivation {
return every.call(canDeactivate$, (result: any) => result === true);
}

private runResolve(future: ActivatedRouteSnapshot): Observable<any> {
private runResolve(
future: ActivatedRouteSnapshot,
paramsInheritanceStrategy: ParamsInheritanceStrategy): Observable<any> {
const resolve = future._resolve;
return map.call(this.resolveNode(resolve, future), (resolvedData: any): any => {
future._resolvedData = resolvedData;
future.data = {...future.data, ...inheritedParamsDataResolve(future).resolve};
future.data = {...future.data,
...inheritedParamsDataResolve(future, paramsInheritanceStrategy).resolve};
return null;
});
}
Expand Down
14 changes: 8 additions & 6 deletions packages/router/src/recognize.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {Observer} from 'rxjs/Observer';
import {of } from 'rxjs/observable/of';

import {Data, ResolveData, Route, Routes} from './config';
import {ActivatedRouteSnapshot, RouterStateSnapshot, inheritedParamsDataResolve} from './router_state';
import {ActivatedRouteSnapshot, ParamsInheritanceStrategy, RouterStateSnapshot, inheritedParamsDataResolve} from './router_state';
import {PRIMARY_OUTLET, defaultUrlMatcher} from './shared';
import {UrlSegment, UrlSegmentGroup, UrlTree, mapChildrenIntoArray} from './url_tree';
import {forEach, last} from './utils/collection';
Expand All @@ -21,15 +21,17 @@ import {TreeNode} from './utils/tree';
class NoMatch {}

export function recognize(
rootComponentType: Type<any>| null, config: Routes, urlTree: UrlTree,
url: string): Observable<RouterStateSnapshot> {
return new Recognizer(rootComponentType, config, urlTree, url).recognize();
rootComponentType: Type<any>| null, config: Routes, urlTree: UrlTree, url: string,
paramsInheritanceStrategy: ParamsInheritanceStrategy =
'emptyOnly'): Observable<RouterStateSnapshot> {
return new Recognizer(rootComponentType, config, urlTree, url, paramsInheritanceStrategy)
.recognize();
}

class Recognizer {
constructor(
private rootComponentType: Type<any>|null, private config: Routes, private urlTree: UrlTree,
private url: string) {}
private url: string, private paramsInheritanceStrategy: ParamsInheritanceStrategy) {}

recognize(): Observable<RouterStateSnapshot> {
try {
Expand All @@ -55,7 +57,7 @@ class Recognizer {
inheritParamsAndData(routeNode: TreeNode<ActivatedRouteSnapshot>): void {
const route = routeNode.value;

const i = inheritedParamsDataResolve(route);
const i = inheritedParamsDataResolve(route, this.paramsInheritanceStrategy);
route.params = Object.freeze(i.params);
route.data = Object.freeze(i.data);

Expand Down
17 changes: 14 additions & 3 deletions packages/router/src/router.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {recognize} from './recognize';
import {DefaultRouteReuseStrategy, DetachedRouteHandleInternal, RouteReuseStrategy} from './route_reuse_strategy';
import {RouterConfigLoader} from './router_config_loader';
import {ChildrenOutletContexts} from './router_outlet_context';
import {ActivatedRoute, ActivatedRouteSnapshot, RouterState, RouterStateSnapshot, advanceActivatedRoute, createEmptyState} from './router_state';
import {ActivatedRoute, ActivatedRouteSnapshot, RouterState, RouterStateSnapshot, advanceActivatedRoute, createEmptyState, inheritedParamsDataResolve} from './router_state';
import {Params, isNavigationCancelingError} from './shared';
import {DefaultUrlHandlingStrategy, UrlHandlingStrategy} from './url_handling_strategy';
import {UrlSerializer, UrlTree, containsTree, createEmptyUrlTree} from './url_tree';
Expand Down Expand Up @@ -249,6 +249,16 @@ export class Router {
*/
onSameUrlNavigation: 'reload'|'ignore' = 'ignore';

/**
* Defines how the router merges params, data and resolved data from parent to child
* routes. Available options are:
*
* - `'emptyOnly'`, the default, only inherits parent params for path-less or component-less
* routes.
* - `'always'`, enables unconditional inheritance of parent params.
*/
paramsInheritanceStrategy: 'emptyOnly'|'always' = 'emptyOnly';

/**
* Creates the router service.
*/
Expand Down Expand Up @@ -611,7 +621,8 @@ export class Router {
urlAndSnapshot$ = mergeMap.call(redirectsApplied$, (appliedUrl: UrlTree) => {
return map.call(
recognize(
this.rootComponentType, this.config, appliedUrl, this.serializeUrl(appliedUrl)),
this.rootComponentType, this.config, appliedUrl, this.serializeUrl(appliedUrl),
this.paramsInheritanceStrategy),
(snapshot: any) => {

(this.events as Subject<Event>)
Expand Down Expand Up @@ -667,7 +678,7 @@ export class Router {
if (p.shouldActivate && preActivation.isActivating()) {
this.triggerEvent(
new ResolveStart(id, this.serializeUrl(url), p.appliedUrl, p.snapshot));
return map.call(preActivation.resolveData(), () => {
return map.call(preActivation.resolveData(this.paramsInheritanceStrategy), () => {
this.triggerEvent(
new ResolveEnd(id, this.serializeUrl(url), p.appliedUrl, p.snapshot));
return p;
Expand Down
14 changes: 14 additions & 0 deletions packages/router/src/router_module.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,16 @@ export interface ExtraOptions {
* current URL. Default is 'ignore'.
*/
onSameUrlNavigation?: 'reload'|'ignore';

/**
* Defines how the router merges params, data and resolved data from parent to child
* routes. Available options are:
*
* - `'emptyOnly'`, the default, only inherits parent params for path-less or component-less
* routes.
* - `'always'`, enables unconditional inheritance of parent params.
*/
paramsInheritanceStrategy?: 'emptyOnly'|'always';
}

export function setupRouter(
Expand Down Expand Up @@ -314,6 +324,10 @@ export function setupRouter(
router.onSameUrlNavigation = opts.onSameUrlNavigation;
}

if (opts.paramsInheritanceStrategy) {
router.paramsInheritanceStrategy = opts.paramsInheritanceStrategy;
}

return router;
}

Expand Down
60 changes: 39 additions & 21 deletions packages/router/src/router_state.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {shallowEqual, shallowEqualArrays} from './utils/collection';
import {Tree, TreeNode} from './utils/tree';



/**
* @whatItDoes Represents the state of the router.
*
Expand Down Expand Up @@ -174,36 +175,53 @@ export class ActivatedRoute {
}
}

/** @internal */
export type ParamsInheritanceStrategy = 'emptyOnly' | 'always';

/** @internal */
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;
/**
* Returns the inherited params, data, and resolve for a given route.
* By default, this only inherits values up to the nearest path-less or component-less route.
* @internal
*/
export function inheritedParamsDataResolve(
route: ActivatedRouteSnapshot,
paramsInheritanceStrategy: ParamsInheritanceStrategy = 'emptyOnly'): Inherited {
const pathFromRoot = route.pathFromRoot;

let inheritingStartingFrom = 0;
if (paramsInheritanceStrategy !== 'always') {
inheritingStartingFrom = pathFromRoot.length - 1;

while (inheritingStartingFrom >= 1) {
const current = pathFromRoot[inheritingStartingFrom];
const parent = pathFromRoot[inheritingStartingFrom - 1];
// current route is an empty path => inherits its parent's params and data
if (current.routeConfig && current.routeConfig.path === '') {
inheritingStartingFrom--;

// parent is componentless => current route should inherit its params and data
} else if (!parent.component) {
inheritingStartingFrom--;

} else {
break;
}
}
}

return pathToRoot.slice(inhertingStartingFrom).reduce((res, curr) => {
return flattenInherited(pathFromRoot.slice(inheritingStartingFrom));
}

/** @internal */
function flattenInherited(pathFromRoot: ActivatedRouteSnapshot[]): Inherited {
return pathFromRoot.reduce((res, curr) => {
const params = {...res.params, ...curr.params};
const data = {...res.data, ...curr.data};
const resolve = {...res.resolve, ...curr._resolvedData};
Expand Down Expand Up @@ -352,7 +370,7 @@ function setRouterState<U, T extends{_routerState: U}>(state: U, node: TreeNode<
}

function serializeNode(node: TreeNode<ActivatedRouteSnapshot>): string {
const c = node.children.length > 0 ? ` { ${node.children.map(serializeNode).join(", ")} } ` : '';
const c = node.children.length > 0 ? ` { ${node.children.map(serializeNode).join(', ')} } ` : '';
return `${node.value}${c}`;
}

Expand Down
13 changes: 13 additions & 0 deletions packages/router/test/integration.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3794,6 +3794,19 @@ describe('Integration', () => {
});
});

describe('Testing router options', () => {
describe('paramsInheritanceStrategy', () => {
beforeEach(() => {
TestBed.configureTestingModule(
{imports: [RouterTestingModule.withRoutes([], {paramsInheritanceStrategy: 'always'})]});
});

it('should configure the router', fakeAsync(inject([Router], (router: Router) => {
expect(router.paramsInheritanceStrategy).toEqual('always');
})));
});
});

function expectEvents(events: Event[], pairs: any[]) {
expect(events.length).toEqual(pairs.length);
for (let i = 0; i < events.length; ++i) {
Expand Down
62 changes: 56 additions & 6 deletions packages/router/test/recognize.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import {Routes} from '../src/config';
import {recognize} from '../src/recognize';
import {ActivatedRouteSnapshot, RouterStateSnapshot} from '../src/router_state';
import {ActivatedRouteSnapshot, ParamsInheritanceStrategy, RouterStateSnapshot, inheritedParamsDataResolve} from '../src/router_state';
import {PRIMARY_OUTLET, Params} from '../src/shared';
import {DefaultUrlSerializer, UrlTree} from '../src/url_tree';

Expand Down Expand Up @@ -201,7 +201,7 @@ describe('recognize', () => {
});
});

it('should merge componentless route\'s data', () => {
it('should inherit componentless route\'s data', () => {
checkRecognize(
[{
path: 'a',
Expand All @@ -214,6 +214,34 @@ describe('recognize', () => {
});
});

it('should not inherit route\'s data if it has component', () => {
checkRecognize(
[{
path: 'a',
component: ComponentA,
data: {one: 1},
children: [{path: 'b', data: {two: 2}, component: ComponentB}]
}],
'a/b', (s: RouterStateSnapshot) => {
const r: ActivatedRouteSnapshot = s.firstChild(<any>s.firstChild(s.root)) !;
expect(r.data).toEqual({two: 2});
});
});

it('should inherit route\'s data if paramsInheritanceStrategy is \'always\'', () => {
checkRecognize(
[{
path: 'a',
component: ComponentA,
data: {one: 1},
children: [{path: 'b', data: {two: 2}, component: ComponentB}]
}],
'a/b', (s: RouterStateSnapshot) => {
const r: ActivatedRouteSnapshot = s.firstChild(<any>s.firstChild(s.root)) !;
expect(r.data).toEqual({one: 1, two: 2});
}, 'always');
});

it('should set resolved data', () => {
checkRecognize(
[{path: 'a', resolve: {one: 'some-token'}, component: ComponentA}], 'a',
Expand Down Expand Up @@ -307,7 +335,7 @@ describe('recognize', () => {
});
});

it('should match (non-termianl) when both primary and secondary and primary has a child',
it('should match (non-terminal) when both primary and secondary and primary has a child',
() => {
const config = [{
path: 'parent',
Expand Down Expand Up @@ -579,7 +607,7 @@ describe('recognize', () => {
});
});

it('should merge params until encounters a normal route', () => {
it('should inherit params until encounters a normal route', () => {
checkRecognize(
[{
path: 'p/:id',
Expand All @@ -606,6 +634,25 @@ describe('recognize', () => {
checkActivatedRoute(c, 'c', {}, ComponentC);
});
});

it('should inherit all params if paramsInheritanceStrategy is \'always\'', () => {
checkRecognize(
[{
path: 'p/:id',
children: [{
path: 'a/:name',
children: [{
path: 'b',
component: ComponentB,
children: [{path: 'c', component: ComponentC}]
}]
}]
}],
'p/11/a/victor/b/c', (s: RouterStateSnapshot) => {
const c = s.firstChild(s.firstChild(s.firstChild(s.firstChild(s.root) !) !) !) !;
checkActivatedRoute(c, 'c', {id: '11', name: 'victor'}, ComponentC);
}, 'always');
});
});

describe('empty URL leftovers', () => {
Expand Down Expand Up @@ -722,8 +769,11 @@ describe('recognize', () => {
});
});

function checkRecognize(config: Routes, url: string, callback: any): void {
recognize(RootComponent, config, tree(url), url).subscribe(callback, e => { throw e; });
function checkRecognize(
config: Routes, url: string, callback: any,
paramsInheritanceStrategy?: ParamsInheritanceStrategy): void {
recognize(RootComponent, config, tree(url), url, paramsInheritanceStrategy)
.subscribe(callback, e => { throw e; });
}

function checkActivatedRoute(
Expand Down
2 changes: 1 addition & 1 deletion packages/router/test/router.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ function checkResolveData(
future: RouterStateSnapshot, curr: RouterStateSnapshot, injector: any, check: any): void {
const p = new PreActivation(future, curr, injector);
p.initialize(new ChildrenOutletContexts());
p.resolveData().subscribe(check, (e) => { throw e; });
p.resolveData('emptyOnly').subscribe(check, (e) => { throw e; });
}

function checkGuards(
Expand Down
Loading

3 comments on commit 5efea2f

@thadguidry
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you say "always" its also inheriting the data also, right ? Any way to inherit only the params, but not inherit the data ?

@lansana
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not work for the inverse. Any way to get all params of the router from the parent route, as opposed to inheriting all params as a child route?

Only way now is to loop until route.firstChild is null or undefined and get all the child params that way... but I would like a clean approach like the one in this commit. That would only make sense given you've done the inverse..

@1antares1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zaven Muradyan, thanks for your effort!

Awesome work! (Y)

Please sign in to comment.