Skip to content

Commit

Permalink
fix(router): Fix _lastPathIndex in deeply nested empty paths (#22394)
Browse files Browse the repository at this point in the history
PR Close #22394
  • Loading branch information
adriensamson authored and vicb committed Jul 25, 2018
1 parent 1e28495 commit 968f153
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 13 deletions.
34 changes: 22 additions & 12 deletions packages/router/src/recognize.ts
Expand Up @@ -20,20 +20,24 @@ class NoMatch {}

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

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

recognize(): Observable<RouterStateSnapshot> {
try {
const rootSegmentGroup = split(this.urlTree.root, [], [], this.config).segmentGroup;
const rootSegmentGroup =
split(this.urlTree.root, [], [], this.config, this.relativeLinkResolution).segmentGroup;

const children = this.processSegmentGroup(this.config, rootSegmentGroup, PRIMARY_OUTLET);

Expand Down Expand Up @@ -134,8 +138,8 @@ class Recognizer {

const childConfig: Route[] = getChildConfig(route);

const {segmentGroup, slicedSegments} =
split(rawSegment, consumedSegments, rawSlicedSegments, childConfig);
const {segmentGroup, slicedSegments} = split(
rawSegment, consumedSegments, rawSlicedSegments, childConfig, this.relativeLinkResolution);

if (slicedSegments.length === 0 && segmentGroup.hasChildren()) {
const children = this.processChildren(childConfig, segmentGroup);
Expand Down Expand Up @@ -232,7 +236,7 @@ function getPathIndexShift(segmentGroup: UrlSegmentGroup): number {

function split(
segmentGroup: UrlSegmentGroup, consumedSegments: UrlSegment[], slicedSegments: UrlSegment[],
config: Route[]) {
config: Route[], relativeLinkResolution: 'legacy' | 'corrected') {
if (slicedSegments.length > 0 &&
containsEmptyPathMatchesWithNamedOutlets(segmentGroup, slicedSegments, config)) {
const s = new UrlSegmentGroup(
Expand All @@ -248,7 +252,8 @@ function split(
containsEmptyPathMatches(segmentGroup, slicedSegments, config)) {
const s = new UrlSegmentGroup(
segmentGroup.segments, addEmptyPathsToChildrenIfNeeded(
segmentGroup, slicedSegments, config, segmentGroup.children));
segmentGroup, consumedSegments, slicedSegments, config,
segmentGroup.children, relativeLinkResolution));
s._sourceSegment = segmentGroup;
s._segmentIndexShift = consumedSegments.length;
return {segmentGroup: s, slicedSegments};
Expand All @@ -261,14 +266,19 @@ function split(
}

function addEmptyPathsToChildrenIfNeeded(
segmentGroup: UrlSegmentGroup, slicedSegments: UrlSegment[], routes: Route[],
children: {[name: string]: UrlSegmentGroup}): {[name: string]: UrlSegmentGroup} {
segmentGroup: UrlSegmentGroup, consumedSegments: UrlSegment[], slicedSegments: UrlSegment[],
routes: Route[], children: {[name: string]: UrlSegmentGroup},
relativeLinkResolution: 'legacy' | 'corrected'): {[name: string]: UrlSegmentGroup} {
const res: {[name: string]: UrlSegmentGroup} = {};
for (const r of routes) {
if (emptyPathMatch(segmentGroup, slicedSegments, r) && !children[getOutlet(r)]) {
const s = new UrlSegmentGroup([], {});
s._sourceSegment = segmentGroup;
s._segmentIndexShift = segmentGroup.segments.length;
if (relativeLinkResolution === 'legacy') {
s._segmentIndexShift = segmentGroup.segments.length;
} else {
s._segmentIndexShift = consumedSegments.length;
}
res[getOutlet(r)] = s;
}
}
Expand Down
7 changes: 6 additions & 1 deletion packages/router/src/router.ts
Expand Up @@ -297,6 +297,11 @@ export class Router {
*/
urlUpdateStrategy: 'deferred'|'eager' = 'deferred';

/**
* See {@link RouterModule} for more information.
*/
relativeLinkResolution: 'legacy'|'corrected' = 'legacy';

/**
* Creates the router service.
*/
Expand Down Expand Up @@ -676,7 +681,7 @@ export class Router {
urlAndSnapshot$ = redirectsApplied$.pipe(mergeMap((appliedUrl: UrlTree) => {
return recognize(
this.rootComponentType, this.config, appliedUrl, this.serializeUrl(appliedUrl),
this.paramsInheritanceStrategy)
this.paramsInheritanceStrategy, this.relativeLinkResolution)
.pipe(map((snapshot: any) => {
(this.events as Subject<Event>)
.next(new RoutesRecognized(
Expand Down
34 changes: 34 additions & 0 deletions packages/router/src/router_module.ts
Expand Up @@ -417,6 +417,36 @@ export interface ExtraOptions {
* - `'eager'`, updates browser URL at the beginning of navigation.
*/
urlUpdateStrategy?: 'deferred'|'eager';

/**
* Enables a bug fix that corrects relative link resolution in components with empty paths.
* Example:
*
* ```
* const routes = [
* {
* path: '',
* component: ContainerComponent,
* children: [
* { path: 'a', component: AComponent },
* { path: 'b', component: BComponent },
* ]
* }
* ];
* ```
*
* From the `ContainerComponent`, this will not work:
*
* `<a [routerLink]="['./a']">Link to A</a>`
*
* However, this will work:
*
* `<a [routerLink]="['../a']">Link to A</a>`
*
* In other words, you're required to use `../` rather than `./`. The current default in v6
* is `legacy`, and this option will be removed in v7 to default to the corrected behavior.
*/
relativeLinkResolution?: 'legacy'|'corrected';
}

export function setupRouter(
Expand Down Expand Up @@ -465,6 +495,10 @@ export function setupRouter(
router.urlUpdateStrategy = opts.urlUpdateStrategy;
}

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

return router;
}

Expand Down
39 changes: 39 additions & 0 deletions packages/router/test/recognize.spec.ts
Expand Up @@ -452,6 +452,45 @@ describe('recognize', () => {
});
});

it('should set url segment and index properly with the "corrected" option for nested empty-path segments',
() => {
const url = tree('a/b') as any;
recognize(
RootComponent, [{
path: 'a',
children: [{
path: 'b',
component: ComponentB,
children: [{
path: '',
component: ComponentC,
children: [{path: '', component: ComponentD}]
}]
}]
}],
url, 'a/b', 'emptyOnly', 'corrected')
.forEach((s: any) => {
expect(s.root._urlSegment).toBe(url.root);
expect(s.root._lastPathIndex).toBe(-1);

const a = s.firstChild(s.root) !;
expect(a._urlSegment).toBe(url.root.children[PRIMARY_OUTLET]);
expect(a._lastPathIndex).toBe(0);

const b = s.firstChild(a) !;
expect(b._urlSegment).toBe(url.root.children[PRIMARY_OUTLET]);
expect(b._lastPathIndex).toBe(1);

const c = s.firstChild(b) !;
expect(c._urlSegment).toBe(url.root.children[PRIMARY_OUTLET]);
expect(c._lastPathIndex).toBe(1);

const d = s.firstChild(c) !;
expect(d._urlSegment).toBe(url.root.children[PRIMARY_OUTLET]);
expect(d._lastPathIndex).toBe(1);
});
});

it('should set url segment and index properly when nested empty-path segments (2)', () => {
const url = tree('');
recognize(
Expand Down
2 changes: 2 additions & 0 deletions tools/public_api_guard/router/router.d.ts
Expand Up @@ -119,6 +119,7 @@ export interface ExtraOptions {
onSameUrlNavigation?: 'reload' | 'ignore';
paramsInheritanceStrategy?: 'emptyOnly' | 'always';
preloadingStrategy?: any;
relativeLinkResolution?: 'legacy' | 'corrected';
scrollOffset?: [number, number] | (() => [number, number]);
scrollPositionRestoration?: 'disabled' | 'enabled' | 'top';
urlUpdateStrategy?: 'deferred' | 'eager';
Expand Down Expand Up @@ -320,6 +321,7 @@ export declare class Router {
navigated: boolean;
onSameUrlNavigation: 'reload' | 'ignore';
paramsInheritanceStrategy: 'emptyOnly' | 'always';
relativeLinkResolution: 'legacy' | 'corrected';
routeReuseStrategy: RouteReuseStrategy;
readonly routerState: RouterState;
readonly url: string;
Expand Down

0 comments on commit 968f153

Please sign in to comment.