From 2c110931f820dab78526058ba9aaaf20cc5e10fa Mon Sep 17 00:00:00 2001 From: vsavkin Date: Tue, 8 Nov 2016 13:36:59 -0800 Subject: [PATCH] fix(router): do not require the creation of empty-path routes when no url left Closes #12133 --- .../@angular/router/src/apply_redirects.ts | 11 +++++- modules/@angular/router/src/recognize.ts | 11 +++++- .../router/test/apply_redirects.spec.ts | 31 ++++++++++++++++ .../@angular/router/test/integration.spec.ts | 36 ++++++++++++++----- .../@angular/router/test/recognize.spec.ts | 28 +++++++++++++++ 5 files changed, 107 insertions(+), 10 deletions(-) diff --git a/modules/@angular/router/src/apply_redirects.ts b/modules/@angular/router/src/apply_redirects.ts index b66afa0a5f..164a8af1bd 100644 --- a/modules/@angular/router/src/apply_redirects.ts +++ b/modules/@angular/router/src/apply_redirects.ts @@ -146,13 +146,22 @@ class ApplyRedirects { const first$ = first.call(concattedProcessedRoutes$, (s: any) => !!s); return _catch.call(first$, (e: any, _: any): Observable => { 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 { diff --git a/modules/@angular/router/src/recognize.ts b/modules/@angular/router/src/recognize.ts index 6e3c7a01de..4fb4fed459 100644 --- a/modules/@angular/router/src/recognize.ts +++ b/modules/@angular/router/src/recognize.ts @@ -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( diff --git a/modules/@angular/router/test/apply_redirects.spec.ts b/modules/@angular/router/test/apply_redirects.spec.ts index c7a2c3a38b..ef96cf017e 100644 --- a/modules/@angular/router/test/apply_redirects.spec.ts +++ b/modules/@angular/router/test/apply_redirects.spec.ts @@ -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 { diff --git a/modules/@angular/router/test/integration.spec.ts b/modules/@angular/router/test/integration.spec.ts index 0b02b88160..ad1867be3c 100644 --- a/modules/@angular/router/test/integration.spec.ts +++ b/modules/@angular/router/test/integration.spec.ts @@ -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] }); }); @@ -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); @@ -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', diff --git a/modules/@angular/router/test/recognize.spec.ts b/modules/@angular/router/test/recognize.spec.ts index 43408f4925..e822d13a52 100644 --- a/modules/@angular/router/test/recognize.spec.ts +++ b/modules/@angular/router/test/recognize.spec.ts @@ -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}];