Skip to content

Commit

Permalink
fix(router): do not require the creation of empty-path routes when no…
Browse files Browse the repository at this point in the history
… url left

Closes #12133
  • Loading branch information
vsavkin authored and vicb committed Nov 10, 2016
1 parent 2ced2a8 commit 2c11093
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 10 deletions.
11 changes: 10 additions & 1 deletion modules/@angular/router/src/apply_redirects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,22 @@ class ApplyRedirects {
const first$ = first.call(concattedProcessedRoutes$, (s: any) => !!s);
return _catch.call(first$, (e: any, _: any): Observable<UrlSegmentGroup> => {
if (e instanceof EmptyError) {
throw new NoMatch(segmentGroup);
if (this.noLeftoversInUrl(segmentGroup, segments, outlet)) {
return of (new UrlSegmentGroup([], {}));
} else {
throw new NoMatch(segmentGroup);
}
} else {
throw e;
}
});
}

private noLeftoversInUrl(segmentGroup: UrlSegmentGroup, segments: UrlSegment[], outlet: string):
boolean {
return segments.length === 0 && !segmentGroup.children[outlet];
}

private expandSegmentAgainstRoute(
injector: Injector, segmentGroup: UrlSegmentGroup, routes: Route[], route: Route,
paths: UrlSegment[], outlet: string, allowRedirects: boolean): Observable<UrlSegmentGroup> {
Expand Down
11 changes: 10 additions & 1 deletion modules/@angular/router/src/recognize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,16 @@ class Recognizer {
if (!(e instanceof NoMatch)) throw e;
}
}
throw new NoMatch();
if (this.noLeftoversInUrl(segmentGroup, segments, outlet)) {
return [];
} else {
throw new NoMatch();
}
}

private noLeftoversInUrl(segmentGroup: UrlSegmentGroup, segments: UrlSegment[], outlet: string):
boolean {
return segments.length === 0 && !segmentGroup.children[outlet];
}

processSegmentAgainstRoute(
Expand Down
31 changes: 31 additions & 0 deletions modules/@angular/router/test/apply_redirects.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,37 @@ describe('applyRedirects', () => {
});
});
});

describe('empty URL leftovers', () => {
it('should not error when no children matching and no url is left', () => {
checkRedirect(
[{path: 'a', component: ComponentA, children: [{path: 'b', component: ComponentB}]}],
'/a', (t: UrlTree) => { compareTrees(t, tree('a')); });
});

it('should not error when no children matching and no url is left (aux routes)', () => {
checkRedirect(
[{
path: 'a',
component: ComponentA,
children: [
{path: 'b', component: ComponentB},
{path: '', redirectTo: 'c', outlet: 'aux'},
{path: 'c', component: ComponentC, outlet: 'aux'},
]
}],
'/a', (t: UrlTree) => { compareTrees(t, tree('a/(aux:c)')); });
});

it('should error when no children matching and some url is left', () => {
applyRedirects(
null, null, tree('/a/c'),
[{path: 'a', component: ComponentA, children: [{path: 'b', component: ComponentB}]}])
.subscribe(
(_) => { throw 'Should not be reached'; },
e => { expect(e.message).toEqual('Cannot match any routes. URL Segment: \'a/c\''); });
});
});
});

function checkRedirect(config: Routes, url: string, callback: any): void {
Expand Down
36 changes: 28 additions & 8 deletions modules/@angular/router/test/integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,8 @@ import {RouterTestingModule, SpyNgModuleFactoryLoader} from '../testing';
describe('Integration', () => {
beforeEach(() => {
TestBed.configureTestingModule({
imports: [
RouterTestingModule.withRoutes(
[{path: '', component: BlankCmp}, {path: 'simple', component: SimpleCmp}]),
TestModule
]
imports:
[RouterTestingModule.withRoutes([{path: 'simple', component: SimpleCmp}]), TestModule]
});
});

Expand Down Expand Up @@ -165,6 +162,29 @@ describe('Integration', () => {
})));
});

it('should not error when no url left and no children are matching',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = createRoot(router, RootCmp);

router.resetConfig([{
path: 'team/:id',
component: TeamCmp,
children: [{path: 'simple', component: SimpleCmp}]
}]);

router.navigateByUrl('/team/33/simple');
advance(fixture);

expect(location.path()).toEqual('/team/33/simple');
expect(fixture.nativeElement).toHaveText('team 33 [ simple, right: ]');

router.navigateByUrl('/team/33');
advance(fixture);

expect(location.path()).toEqual('/team/33');
expect(fixture.nativeElement).toHaveText('team 33 [ , right: ]');
})));

it('should work when an outlet is in an ngIf',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = createRoot(router, RootCmp);
Expand Down Expand Up @@ -711,13 +731,13 @@ describe('Integration', () => {
expect(cmp.activations.length).toEqual(1);
expect(cmp.activations[0] instanceof BlankCmp).toBe(true);

router.navigateByUrl('/simple');
router.navigateByUrl('/simple').catch(e => console.log(e));
advance(fixture);

expect(cmp.activations.length).toEqual(2);
expect(cmp.activations[1] instanceof SimpleCmp).toBe(true);
expect(cmp.deactivations.length).toEqual(2);
expect(cmp.deactivations[1] instanceof BlankCmp).toBe(true);
expect(cmp.deactivations.length).toEqual(1);
expect(cmp.deactivations[0] instanceof BlankCmp).toBe(true);
}));

it('should update url and router state before activating components',
Expand Down
28 changes: 28 additions & 0 deletions modules/@angular/router/test/recognize.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,34 @@ describe('recognize', () => {
});
});

describe('empty URL leftovers', () => {
it('should not throw when no children matching', () => {
checkRecognize(
[{path: 'a', component: ComponentA, children: [{path: 'b', component: ComponentB}]}],
'/a', (s: RouterStateSnapshot) => {
const a = s.firstChild(s.root);
checkActivatedRoute(a, 'a', {}, ComponentA);
});
});

it('should not throw when no children matching (aux routes)', () => {
checkRecognize(
[{
path: 'a',
component: ComponentA,
children: [
{path: 'b', component: ComponentB},
{path: '', component: ComponentC, outlet: 'aux'},
]
}],
'/a', (s: RouterStateSnapshot) => {
const a = s.firstChild(s.root);
checkActivatedRoute(a, 'a', {}, ComponentA);
checkActivatedRoute(a.children[0], '', {}, ComponentC, 'aux');
});
});
});

describe('query parameters', () => {
it('should support query params', () => {
const config = [{path: 'a', component: ComponentA}];
Expand Down

0 comments on commit 2c11093

Please sign in to comment.