Skip to content

Commit d8c0534

Browse files
zelliottjosephperrott
authored andcommitted
fix(router): properly assign ExtraOptions to Router in RouterTestingModule (angular#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 angular#23347 PR Close angular#39096
1 parent faa81dc commit d8c0534

File tree

4 files changed

+59
-38
lines changed

4 files changed

+59
-38
lines changed

packages/router/src/private_export.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@
88

99

1010
export {ɵEmptyOutletComponent} from './components/empty_outlet';
11-
export {ROUTER_PROVIDERS as ɵROUTER_PROVIDERS} from './router_module';
11+
export {assignExtraOptionsToRouter as ɵassignExtraOptionsToRouter, ROUTER_PROVIDERS as ɵROUTER_PROVIDERS} from './router_module';
1212
export {flatten as ɵflatten} from './utils/collection';

packages/router/src/router_module.ts

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -448,13 +448,7 @@ export function setupRouter(
448448
router.routeReuseStrategy = routeReuseStrategy;
449449
}
450450

451-
if (opts.errorHandler) {
452-
router.errorHandler = opts.errorHandler;
453-
}
454-
455-
if (opts.malformedUriErrorHandler) {
456-
router.malformedUriErrorHandler = opts.malformedUriErrorHandler;
457-
}
451+
assignExtraOptionsToRouter(opts, router);
458452

459453
if (opts.enableTracing) {
460454
const dom = getDOM();
@@ -466,6 +460,18 @@ export function setupRouter(
466460
});
467461
}
468462

463+
return router;
464+
}
465+
466+
export function assignExtraOptionsToRouter(opts: ExtraOptions, router: Router): void {
467+
if (opts.errorHandler) {
468+
router.errorHandler = opts.errorHandler;
469+
}
470+
471+
if (opts.malformedUriErrorHandler) {
472+
router.malformedUriErrorHandler = opts.malformedUriErrorHandler;
473+
}
474+
469475
if (opts.onSameUrlNavigation) {
470476
router.onSameUrlNavigation = opts.onSameUrlNavigation;
471477
}
@@ -474,15 +480,13 @@ export function setupRouter(
474480
router.paramsInheritanceStrategy = opts.paramsInheritanceStrategy;
475481
}
476482

477-
if (opts.urlUpdateStrategy) {
478-
router.urlUpdateStrategy = opts.urlUpdateStrategy;
479-
}
480-
481483
if (opts.relativeLinkResolution) {
482484
router.relativeLinkResolution = opts.relativeLinkResolution;
483485
}
484486

485-
return router;
487+
if (opts.urlUpdateStrategy) {
488+
router.urlUpdateStrategy = opts.urlUpdateStrategy;
489+
}
486490
}
487491

488492
export function rootRoute(router: Router): ActivatedRoute {

packages/router/test/integration.spec.ts

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5625,30 +5625,54 @@ describe('Integration', () => {
56255625
});
56265626

56275627
describe('Testing router options', () => {
5628-
describe('paramsInheritanceStrategy', () => {
5629-
beforeEach(() => {
5628+
describe('should configure the router', () => {
5629+
it('assigns errorHandler', () => {
5630+
function errorHandler(error: any) {
5631+
throw error;
5632+
}
56305633
TestBed.configureTestingModule(
5631-
{imports: [RouterTestingModule.withRoutes([], {paramsInheritanceStrategy: 'always'})]});
5634+
{imports: [RouterTestingModule.withRoutes([], {errorHandler})]});
5635+
const router: Router = TestBed.inject(Router);
5636+
expect(router.errorHandler).toBe(errorHandler);
56325637
});
56335638

5634-
it('should configure the router', fakeAsync(inject([Router], (router: Router) => {
5635-
expect(router.paramsInheritanceStrategy).toEqual('always');
5636-
})));
5637-
});
5639+
it('assigns malformedUriErrorHandler', () => {
5640+
function malformedUriErrorHandler(e: URIError, urlSerializer: UrlSerializer, url: string) {
5641+
return urlSerializer.parse('/error');
5642+
}
5643+
TestBed.configureTestingModule(
5644+
{imports: [RouterTestingModule.withRoutes([], {malformedUriErrorHandler})]});
5645+
const router: Router = TestBed.inject(Router);
5646+
expect(router.malformedUriErrorHandler).toBe(malformedUriErrorHandler);
5647+
});
56385648

5639-
describe('malformedUriErrorHandler', () => {
5640-
function malformedUriErrorHandler(e: URIError, urlSerializer: UrlSerializer, url: string) {
5641-
return urlSerializer.parse('/error');
5642-
}
5649+
it('assigns onSameUrlNavigation', () => {
5650+
TestBed.configureTestingModule(
5651+
{imports: [RouterTestingModule.withRoutes([], {onSameUrlNavigation: 'reload'})]});
5652+
const router: Router = TestBed.inject(Router);
5653+
expect(router.onSameUrlNavigation).toBe('reload');
5654+
});
56435655

5644-
beforeEach(() => {
5656+
it('assigns paramsInheritanceStrategy', () => {
56455657
TestBed.configureTestingModule(
5646-
{imports: [RouterTestingModule.withRoutes([], {malformedUriErrorHandler})]});
5658+
{imports: [RouterTestingModule.withRoutes([], {paramsInheritanceStrategy: 'always'})]});
5659+
const router: Router = TestBed.inject(Router);
5660+
expect(router.paramsInheritanceStrategy).toBe('always');
56475661
});
56485662

5649-
it('should configure the router', fakeAsync(inject([Router], (router: Router) => {
5650-
expect(router.malformedUriErrorHandler).toBe(malformedUriErrorHandler);
5651-
})));
5663+
it('assigns relativeLinkResolution', () => {
5664+
TestBed.configureTestingModule(
5665+
{imports: [RouterTestingModule.withRoutes([], {relativeLinkResolution: 'corrected'})]});
5666+
const router: Router = TestBed.inject(Router);
5667+
expect(router.relativeLinkResolution).toBe('corrected');
5668+
});
5669+
5670+
it('assigns urlUpdateStrategy', () => {
5671+
TestBed.configureTestingModule(
5672+
{imports: [RouterTestingModule.withRoutes([], {urlUpdateStrategy: 'eager'})]});
5673+
const router: Router = TestBed.inject(Router);
5674+
expect(router.urlUpdateStrategy).toBe('eager');
5675+
});
56525676
});
56535677
});
56545678

packages/router/testing/src/router_testing_module.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import {Location, LocationStrategy} from '@angular/common';
1010
import {MockLocationStrategy, SpyLocation} from '@angular/common/testing';
1111
import {Compiler, Injectable, Injector, ModuleWithProviders, NgModule, NgModuleFactory, NgModuleFactoryLoader, Optional} from '@angular/core';
12-
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';
12+
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';
1313

1414

1515

@@ -124,14 +124,7 @@ export function setupTestingRouter(
124124
router.urlHandlingStrategy = opts;
125125
} else {
126126
// Handle ExtraOptions
127-
128-
if (opts.malformedUriErrorHandler) {
129-
router.malformedUriErrorHandler = opts.malformedUriErrorHandler;
130-
}
131-
132-
if (opts.paramsInheritanceStrategy) {
133-
router.paramsInheritanceStrategy = opts.paramsInheritanceStrategy;
134-
}
127+
assignExtraOptionsToRouter(opts, router);
135128
}
136129
}
137130

0 commit comments

Comments
 (0)