fix(router): do not require the creation of empty-path routes when no url left
Closes #12133
This commit is contained in:
		
							parent
							
								
									2ced2a8a5a
								
							
						
					
					
						commit
						2c110931f8
					
				| @ -146,13 +146,22 @@ class ApplyRedirects { | |||||||
|     const first$ = first.call(concattedProcessedRoutes$, (s: any) => !!s); |     const first$ = first.call(concattedProcessedRoutes$, (s: any) => !!s); | ||||||
|     return _catch.call(first$, (e: any, _: any): Observable<UrlSegmentGroup> => { |     return _catch.call(first$, (e: any, _: any): Observable<UrlSegmentGroup> => { | ||||||
|       if (e instanceof EmptyError) { |       if (e instanceof EmptyError) { | ||||||
|         throw new NoMatch(segmentGroup); |         if (this.noLeftoversInUrl(segmentGroup, segments, outlet)) { | ||||||
|  |           return of (new UrlSegmentGroup([], {})); | ||||||
|  |         } else { | ||||||
|  |           throw new NoMatch(segmentGroup); | ||||||
|  |         } | ||||||
|       } else { |       } else { | ||||||
|         throw e; |         throw e; | ||||||
|       } |       } | ||||||
|     }); |     }); | ||||||
|   } |   } | ||||||
| 
 | 
 | ||||||
|  |   private noLeftoversInUrl(segmentGroup: UrlSegmentGroup, segments: UrlSegment[], outlet: string): | ||||||
|  |       boolean { | ||||||
|  |     return segments.length === 0 && !segmentGroup.children[outlet]; | ||||||
|  |   } | ||||||
|  | 
 | ||||||
|   private expandSegmentAgainstRoute( |   private expandSegmentAgainstRoute( | ||||||
|       injector: Injector, segmentGroup: UrlSegmentGroup, routes: Route[], route: Route, |       injector: Injector, segmentGroup: UrlSegmentGroup, routes: Route[], route: Route, | ||||||
|       paths: UrlSegment[], outlet: string, allowRedirects: boolean): Observable<UrlSegmentGroup> { |       paths: UrlSegment[], outlet: string, allowRedirects: boolean): Observable<UrlSegmentGroup> { | ||||||
|  | |||||||
| @ -90,7 +90,16 @@ class Recognizer { | |||||||
|         if (!(e instanceof NoMatch)) throw e; |         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( |   processSegmentAgainstRoute( | ||||||
|  | |||||||
| @ -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 { | function checkRedirect(config: Routes, url: string, callback: any): void { | ||||||
|  | |||||||
| @ -22,11 +22,8 @@ import {RouterTestingModule, SpyNgModuleFactoryLoader} from '../testing'; | |||||||
| describe('Integration', () => { | describe('Integration', () => { | ||||||
|   beforeEach(() => { |   beforeEach(() => { | ||||||
|     TestBed.configureTestingModule({ |     TestBed.configureTestingModule({ | ||||||
|       imports: [ |       imports: | ||||||
|         RouterTestingModule.withRoutes( |           [RouterTestingModule.withRoutes([{path: 'simple', component: SimpleCmp}]), TestModule] | ||||||
|             [{path: '', component: BlankCmp}, {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', |   it('should work when an outlet is in an ngIf', | ||||||
|      fakeAsync(inject([Router, Location], (router: Router, location: Location) => { |      fakeAsync(inject([Router, Location], (router: Router, location: Location) => { | ||||||
|        const fixture = createRoot(router, RootCmp); |        const fixture = createRoot(router, RootCmp); | ||||||
| @ -711,13 +731,13 @@ describe('Integration', () => { | |||||||
|        expect(cmp.activations.length).toEqual(1); |        expect(cmp.activations.length).toEqual(1); | ||||||
|        expect(cmp.activations[0] instanceof BlankCmp).toBe(true); |        expect(cmp.activations[0] instanceof BlankCmp).toBe(true); | ||||||
| 
 | 
 | ||||||
|        router.navigateByUrl('/simple'); |        router.navigateByUrl('/simple').catch(e => console.log(e)); | ||||||
|        advance(fixture); |        advance(fixture); | ||||||
| 
 | 
 | ||||||
|        expect(cmp.activations.length).toEqual(2); |        expect(cmp.activations.length).toEqual(2); | ||||||
|        expect(cmp.activations[1] instanceof SimpleCmp).toBe(true); |        expect(cmp.activations[1] instanceof SimpleCmp).toBe(true); | ||||||
|        expect(cmp.deactivations.length).toEqual(2); |        expect(cmp.deactivations.length).toEqual(1); | ||||||
|        expect(cmp.deactivations[1] instanceof BlankCmp).toBe(true); |        expect(cmp.deactivations[0] instanceof BlankCmp).toBe(true); | ||||||
|      })); |      })); | ||||||
| 
 | 
 | ||||||
|   it('should update url and router state before activating components', |   it('should update url and router state before activating components', | ||||||
|  | |||||||
| @ -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', () => { |   describe('query parameters', () => { | ||||||
|     it('should support query params', () => { |     it('should support query params', () => { | ||||||
|       const config = [{path: 'a', component: ComponentA}]; |       const config = [{path: 'a', component: ComponentA}]; | ||||||
|  | |||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user