refactor(router): Eliminate constructor parameters in Router class (#48215)

The Router constructor and `setupRouter` factory mainly exist as a
legacy configuration. Since the Router's creation, the style in Angular
has evolved quite a bit. This commit eliminates and cleans up some
unnecessary comlicated code paths related to the router
constructor/factory.

Note that there are edits to the `setupTestingRouter` that could be seen
as breaking. However, it is not for several reasons:

1. The function is documented as a factory function. If used as
   documented, the parameters should match what's available in DI
2. The function is totally unused by the Router itself and is not used
   in g3 either. I believe it was made publicApi by an error when
   updating documentation annotations long ago.

PR Close #48215
This commit is contained in:
Andrew Scott
2022-11-23 15:07:35 -08:00
committed by Andrew Kushnir
parent 4d398a0bab
commit a0551ee761
4 changed files with 50 additions and 54 deletions
+1 -6
View File
@@ -16,7 +16,6 @@ import { EventEmitter } from '@angular/core';
import * as i0 from '@angular/core';
import { InjectionToken } from '@angular/core';
import { Injector } from '@angular/core';
import { Location as Location_2 } from '@angular/common';
import { LocationStrategy } from '@angular/common';
import { ModuleWithProviders } from '@angular/core';
import { NgModuleFactory } from '@angular/core';
@@ -655,11 +654,7 @@ export class RouteConfigLoadStart {
// @public
export class Router {
constructor(
rootComponentType: Type<any> | null,
urlSerializer: UrlSerializer,
rootContexts: ChildrenOutletContexts,
location: Location_2, injector: Injector, compiler: Compiler, config: Routes);
constructor();
// @deprecated
canceledNavigationResolution: 'replace' | 'computed';
// (undocumented)
+16 -43
View File
@@ -87,22 +87,6 @@ export function assignExtraOptionsToRouter(opts: ExtraOptions, router: Router):
}
}
export function setupRouter() {
const urlSerializer = inject(UrlSerializer);
const contexts = inject(ChildrenOutletContexts);
const location = inject(Location);
const injector = inject(Injector);
const compiler = inject(Compiler);
const config = inject(ROUTES, {optional: true}) ?? [];
const opts = inject(ROUTER_CONFIGURATION, {optional: true}) ?? {};
const router =
new Router(null, urlSerializer, contexts, location, injector, compiler, flatten(config));
assignExtraOptionsToRouter(opts, router);
return router;
}
/**
* @description
*
@@ -115,10 +99,7 @@ export function setupRouter() {
*
* @publicApi
*/
@Injectable({
providedIn: 'root',
useFactory: setupRouter,
})
@Injectable({providedIn: 'root'})
export class Router {
/**
* Represents the activated `UrlTree` that the `Router` is configured to handle (through
@@ -201,7 +182,7 @@ export class Router {
private get browserPageId(): number|undefined {
return (this.location.getState() as RestoredState | null)?.ɵrouterPageId;
}
private console: Console;
private console = inject(Console);
private isNgZoneEnabled: boolean = false;
/**
@@ -349,27 +330,22 @@ export class Router {
*/
canceledNavigationResolution: 'replace'|'computed' = 'replace';
config: Routes = flatten(inject(ROUTES, {optional: true}) ?? []);
private readonly navigationTransitions = inject(NavigationTransitions);
private readonly urlSerializer = inject(UrlSerializer);
private readonly location = inject(Location);
/**
* Creates the router service.
*/
// TODO: vsavkin make internal after the final is out.
constructor(
/** @internal */
public rootComponentType: Type<any>|null,
private readonly urlSerializer: UrlSerializer,
private readonly rootContexts: ChildrenOutletContexts,
private readonly location: Location,
injector: Injector,
compiler: Compiler,
public config: Routes,
) {
this.console = injector.get(Console);
const ngZone = injector.get(NgZone);
this.isNgZoneEnabled = ngZone instanceof NgZone && NgZone.isInAngularZone();
/** @internal */
rootComponentType: Type<any>|null = null;
this.resetConfig(config);
constructor() {
const opts = inject(ROUTER_CONFIGURATION, {optional: true}) ?? {};
assignExtraOptionsToRouter(opts, this);
this.isNgZoneEnabled = inject(NgZone) instanceof NgZone && NgZone.isInAngularZone();
this.resetConfig(this.config);
this.currentUrlTree = new UrlTree();
this.rawUrlTree = this.currentUrlTree;
this.browserUrlTree = this.currentUrlTree;
@@ -386,10 +362,7 @@ export class Router {
});
}
/**
* @internal
* TODO: this should be removed once the constructor of the router made internal
*/
/** @internal */
resetRootComponentType(rootComponentType: Type<any>): void {
this.rootComponentType = rootComponentType;
// TODO: vsavkin router 4.0 should make the root component set to null
+2 -2
View File
@@ -17,7 +17,7 @@ import {RuntimeErrorCode} from './errors';
import {Routes} from './models';
import {NavigationTransitions} from './navigation_transition';
import {getBootstrapListener, rootRoute, ROUTER_IS_PROVIDED, withDebugTracing, withDisabledInitialNavigation, withEnabledBlockingInitialNavigation, withPreloading} from './provide_router';
import {Router, setupRouter} from './router';
import {Router} from './router';
import {ExtraOptions, ROUTER_CONFIGURATION} from './router_config';
import {RouterConfigLoader, ROUTES} from './router_config_loader';
import {ChildrenOutletContexts} from './router_outlet_context';
@@ -45,7 +45,7 @@ export const ROUTER_FORROOT_GUARD = new InjectionToken<void>(
export const ROUTER_PROVIDERS: Provider[] = [
Location,
{provide: UrlSerializer, useClass: DefaultUrlSerializer},
{provide: Router, useFactory: setupRouter},
Router,
ChildrenOutletContexts,
{provide: ActivatedRoute, useFactory: rootRoute, deps: [Router]},
RouterConfigLoader,
@@ -8,7 +8,7 @@
import {Location} from '@angular/common';
import {provideLocationMocks} from '@angular/common/testing';
import {Compiler, Injector, ModuleWithProviders, NgModule, Optional} from '@angular/core';
import {Compiler, inject, Injector, ModuleWithProviders, NgModule} from '@angular/core';
import {ChildrenOutletContexts, ExtraOptions, NoPreloading, Route, Router, ROUTER_CONFIGURATION, RouteReuseStrategy, RouterModule, ROUTES, Routes, TitleStrategy, UrlHandlingStrategy, UrlSerializer, ɵassignExtraOptionsToRouter as assignExtraOptionsToRouter, ɵflatten as flatten, ɵROUTER_PROVIDERS as ROUTER_PROVIDERS, ɵwithPreloading as withPreloading} from '@angular/router';
import {EXTRA_ROUTER_TESTING_PROVIDERS} from './extra_router_testing_providers';
@@ -20,6 +20,12 @@ function isUrlHandlingStrategy(opts: ExtraOptions|
return 'shouldProcessUrl' in opts;
}
function throwInvalidConfigError(parameter: string): never {
throw new Error(
`Parameter ${parameter} does not match the one available in the injector. ` +
'`setupTestingRouter` is meant to be used as a factory function with dependencies coming from DI.');
}
/**
* Router setup factory function used for testing.
*
@@ -31,8 +37,30 @@ export function setupTestingRouter(
compiler: Compiler, injector: Injector, routes: Route[][],
opts?: ExtraOptions|UrlHandlingStrategy|null, urlHandlingStrategy?: UrlHandlingStrategy,
routeReuseStrategy?: RouteReuseStrategy, titleStrategy?: TitleStrategy) {
const router =
new Router(null!, urlSerializer, contexts, location, injector, compiler, flatten(routes));
// Note: The checks below are to detect misconfigured providers and invalid uses of
// `setupTestingRouter`. This function is not used internally (neither in router code or anywhere
// in g3). It appears this function was exposed as publicApi by mistake and should not be used
// externally either. However, if it is, the documented intent is to be used as a factory function
// and parameter values should always match what's available in DI.
const router = new Router();
if (urlSerializer !== inject(UrlSerializer)) {
throwInvalidConfigError('urlSerializer');
}
if (contexts !== inject(ChildrenOutletContexts)) {
throwInvalidConfigError('contexts');
}
if (location !== inject(Location)) {
throwInvalidConfigError('location');
}
if (compiler !== inject(Compiler)) {
throwInvalidConfigError('compiler');
}
if (injector !== inject(Injector)) {
throwInvalidConfigError('injector');
}
if (routes !== inject(ROUTES)) {
throwInvalidConfigError('routes');
}
if (opts) {
// Handle deprecated argument ordering.
if (isUrlHandlingStrategy(opts)) {