From d8c05347c341b2375644aef60baf371dbf02c3c6 Mon Sep 17 00:00:00 2001 From: Zack Elliott Date: Fri, 2 Oct 2020 08:52:40 -0700 Subject: [PATCH] fix(router): properly assign ExtraOptions to Router in RouterTestingModule (#39096) Previously, RouterTestingModule only assigned two of the options within ExtraOptions to the Router. Now, it assigns the same options as RouterModule does (with the exception of enableTracing) via a new shared function assignExtraOptionsToRouter. Fixes #23347 PR Close #39096 --- packages/router/src/private_export.ts | 2 +- packages/router/src/router_module.ts | 28 +++++---- packages/router/test/integration.spec.ts | 58 +++++++++++++------ .../testing/src/router_testing_module.ts | 11 +--- 4 files changed, 60 insertions(+), 39 deletions(-) diff --git a/packages/router/src/private_export.ts b/packages/router/src/private_export.ts index fa90c45ee5..8fe6e90686 100644 --- a/packages/router/src/private_export.ts +++ b/packages/router/src/private_export.ts @@ -8,5 +8,5 @@ export {ɵEmptyOutletComponent} from './components/empty_outlet'; -export {ROUTER_PROVIDERS as ɵROUTER_PROVIDERS} from './router_module'; +export {assignExtraOptionsToRouter as ɵassignExtraOptionsToRouter, ROUTER_PROVIDERS as ɵROUTER_PROVIDERS} from './router_module'; export {flatten as ɵflatten} from './utils/collection'; diff --git a/packages/router/src/router_module.ts b/packages/router/src/router_module.ts index 974839fa36..bf2d216f63 100644 --- a/packages/router/src/router_module.ts +++ b/packages/router/src/router_module.ts @@ -448,13 +448,7 @@ export function setupRouter( router.routeReuseStrategy = routeReuseStrategy; } - if (opts.errorHandler) { - router.errorHandler = opts.errorHandler; - } - - if (opts.malformedUriErrorHandler) { - router.malformedUriErrorHandler = opts.malformedUriErrorHandler; - } + assignExtraOptionsToRouter(opts, router); if (opts.enableTracing) { const dom = getDOM(); @@ -466,6 +460,18 @@ export function setupRouter( }); } + return router; +} + +export function assignExtraOptionsToRouter(opts: ExtraOptions, router: Router): void { + if (opts.errorHandler) { + router.errorHandler = opts.errorHandler; + } + + if (opts.malformedUriErrorHandler) { + router.malformedUriErrorHandler = opts.malformedUriErrorHandler; + } + if (opts.onSameUrlNavigation) { router.onSameUrlNavigation = opts.onSameUrlNavigation; } @@ -474,15 +480,13 @@ export function setupRouter( router.paramsInheritanceStrategy = opts.paramsInheritanceStrategy; } - if (opts.urlUpdateStrategy) { - router.urlUpdateStrategy = opts.urlUpdateStrategy; - } - if (opts.relativeLinkResolution) { router.relativeLinkResolution = opts.relativeLinkResolution; } - return router; + if (opts.urlUpdateStrategy) { + router.urlUpdateStrategy = opts.urlUpdateStrategy; + } } export function rootRoute(router: Router): ActivatedRoute { diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts index 6c5e5e5d89..4884edf007 100644 --- a/packages/router/test/integration.spec.ts +++ b/packages/router/test/integration.spec.ts @@ -5625,30 +5625,54 @@ describe('Integration', () => { }); describe('Testing router options', () => { - describe('paramsInheritanceStrategy', () => { - beforeEach(() => { + describe('should configure the router', () => { + it('assigns errorHandler', () => { + function errorHandler(error: any) { + throw error; + } TestBed.configureTestingModule( - {imports: [RouterTestingModule.withRoutes([], {paramsInheritanceStrategy: 'always'})]}); + {imports: [RouterTestingModule.withRoutes([], {errorHandler})]}); + const router: Router = TestBed.inject(Router); + expect(router.errorHandler).toBe(errorHandler); }); - it('should configure the router', fakeAsync(inject([Router], (router: Router) => { - expect(router.paramsInheritanceStrategy).toEqual('always'); - }))); - }); - - describe('malformedUriErrorHandler', () => { - function malformedUriErrorHandler(e: URIError, urlSerializer: UrlSerializer, url: string) { - return urlSerializer.parse('/error'); - } - - beforeEach(() => { + it('assigns malformedUriErrorHandler', () => { + function malformedUriErrorHandler(e: URIError, urlSerializer: UrlSerializer, url: string) { + return urlSerializer.parse('/error'); + } TestBed.configureTestingModule( {imports: [RouterTestingModule.withRoutes([], {malformedUriErrorHandler})]}); + const router: Router = TestBed.inject(Router); + expect(router.malformedUriErrorHandler).toBe(malformedUriErrorHandler); }); - it('should configure the router', fakeAsync(inject([Router], (router: Router) => { - expect(router.malformedUriErrorHandler).toBe(malformedUriErrorHandler); - }))); + it('assigns onSameUrlNavigation', () => { + TestBed.configureTestingModule( + {imports: [RouterTestingModule.withRoutes([], {onSameUrlNavigation: 'reload'})]}); + const router: Router = TestBed.inject(Router); + expect(router.onSameUrlNavigation).toBe('reload'); + }); + + it('assigns paramsInheritanceStrategy', () => { + TestBed.configureTestingModule( + {imports: [RouterTestingModule.withRoutes([], {paramsInheritanceStrategy: 'always'})]}); + const router: Router = TestBed.inject(Router); + expect(router.paramsInheritanceStrategy).toBe('always'); + }); + + it('assigns relativeLinkResolution', () => { + TestBed.configureTestingModule( + {imports: [RouterTestingModule.withRoutes([], {relativeLinkResolution: 'corrected'})]}); + const router: Router = TestBed.inject(Router); + expect(router.relativeLinkResolution).toBe('corrected'); + }); + + it('assigns urlUpdateStrategy', () => { + TestBed.configureTestingModule( + {imports: [RouterTestingModule.withRoutes([], {urlUpdateStrategy: 'eager'})]}); + const router: Router = TestBed.inject(Router); + expect(router.urlUpdateStrategy).toBe('eager'); + }); }); }); diff --git a/packages/router/testing/src/router_testing_module.ts b/packages/router/testing/src/router_testing_module.ts index 36db3c7a08..e7dc95e23a 100644 --- a/packages/router/testing/src/router_testing_module.ts +++ b/packages/router/testing/src/router_testing_module.ts @@ -9,7 +9,7 @@ import {Location, LocationStrategy} from '@angular/common'; import {MockLocationStrategy, SpyLocation} from '@angular/common/testing'; import {Compiler, Injectable, Injector, ModuleWithProviders, NgModule, NgModuleFactory, NgModuleFactoryLoader, Optional} from '@angular/core'; -import {ChildrenOutletContexts, ExtraOptions, NoPreloading, PreloadingStrategy, provideRoutes, Route, Router, ROUTER_CONFIGURATION, RouterModule, ROUTES, Routes, UrlHandlingStrategy, UrlSerializer, ɵflatten as flatten, ɵROUTER_PROVIDERS as ROUTER_PROVIDERS} from '@angular/router'; +import {ChildrenOutletContexts, ExtraOptions, NoPreloading, PreloadingStrategy, provideRoutes, Route, Router, ROUTER_CONFIGURATION, RouterModule, ROUTES, Routes, UrlHandlingStrategy, UrlSerializer, ɵassignExtraOptionsToRouter as assignExtraOptionsToRouter, ɵflatten as flatten, ɵROUTER_PROVIDERS as ROUTER_PROVIDERS} from '@angular/router'; @@ -124,14 +124,7 @@ export function setupTestingRouter( router.urlHandlingStrategy = opts; } else { // Handle ExtraOptions - - if (opts.malformedUriErrorHandler) { - router.malformedUriErrorHandler = opts.malformedUriErrorHandler; - } - - if (opts.paramsInheritanceStrategy) { - router.paramsInheritanceStrategy = opts.paramsInheritanceStrategy; - } + assignExtraOptionsToRouter(opts, router); } }