Skip to content

Commit

Permalink
refactor(router): Move config loader tracking to the RouterConfigLoad…
Browse files Browse the repository at this point in the history
…er (#45656)

This wasn't exactly possible before because the `RouterConfigLoader` was
not an Injectable so there wasn't a straightforward way to share
information between `ApplyRedirects` and the preloader. They each had
their own implementation so they needed to store the values on the
`Route` so they both had access to them. I imagine this was the case
because trying to inject `Router` (to get access to the events) into the
preloader would have caused a circular dependency.

This refactor co-locates the loading details with the loader itself
rather than leaking implementation into the public route config and
mutating the object in an awkward way. This also promotes
`RouterConfigLoader` to a proper `Injectable` so data can be shared
throughout the system.

PR Close #45656
  • Loading branch information
atscott authored and dylhunn committed Apr 18, 2022
1 parent ec5cb0b commit ea8256f
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 35 deletions.
3 changes: 2 additions & 1 deletion goldens/public-api/router/index.md
Expand Up @@ -10,6 +10,7 @@ import { Compiler } from '@angular/core';
import { ComponentFactoryResolver } from '@angular/core';
import { ComponentRef } from '@angular/core';
import { ElementRef } from '@angular/core';
import { EnvironmentInjector } from '@angular/core';
import { EventEmitter } from '@angular/core';
import * as i0 from '@angular/core';
import { InjectionToken } from '@angular/core';
Expand Down Expand Up @@ -691,7 +692,7 @@ export interface RouterOutletContract {

// @public
export class RouterPreloader implements OnDestroy {
constructor(router: Router, compiler: Compiler, injector: Injector, preloadingStrategy: PreloadingStrategy);
constructor(router: Router, compiler: Compiler, injector: Injector, preloadingStrategy: PreloadingStrategy, loader: RouterConfigLoader);
// (undocumented)
ngOnDestroy(): void;
// (undocumented)
Expand Down
12 changes: 6 additions & 6 deletions packages/core/test/bundling/router/bundle.golden_symbols.json
Expand Up @@ -212,6 +212,9 @@
{
"name": "FilterSubscriber"
},
{
"name": "FinallyOperator"
},
{
"name": "FinallySubscriber"
},
Expand Down Expand Up @@ -512,12 +515,6 @@
{
"name": "RootViewRef"
},
{
"name": "RouteConfigLoadEnd"
},
{
"name": "RouteConfigLoadStart"
},
{
"name": "RouteExampleModule"
},
Expand Down Expand Up @@ -1094,6 +1091,9 @@
{
"name": "filter"
},
{
"name": "finalize"
},
{
"name": "findAttrIndexInNode"
},
Expand Down
6 changes: 0 additions & 6 deletions packages/router/src/models.ts
Expand Up @@ -490,12 +490,6 @@ export interface Route {
* @internal
*/
_loadedInjector?: EnvironmentInjector;

/**
* Filled for routes with `loadChildren` during load
* @internal
*/
_loader$?: Observable<LoadedRouterConfig>;
}

export interface LoadedRouterConfig {
Expand Down
4 changes: 3 additions & 1 deletion packages/router/src/router.ts
Expand Up @@ -594,6 +594,9 @@ export class Router {
compiler: Compiler, public config: Routes) {
const onLoadStart = (r: Route) => this.triggerEvent(new RouteConfigLoadStart(r));
const onLoadEnd = (r: Route) => this.triggerEvent(new RouteConfigLoadEnd(r));
this.configLoader = injector.get(RouterConfigLoader);
this.configLoader.onLoadEndListener = onLoadEnd;
this.configLoader.onLoadStartListener = onLoadStart;

this.ngModule = injector.get(NgModuleRef);
this.console = injector.get(Console);
Expand All @@ -605,7 +608,6 @@ export class Router {
this.rawUrlTree = this.currentUrlTree;
this.browserUrlTree = this.currentUrlTree;

this.configLoader = new RouterConfigLoader(injector, compiler, onLoadStart, onLoadEnd);
this.routerState = createEmptyState(this.currentUrlTree, this.rootComponentType);

this.transitions = new BehaviorSubject<NavigationTransition>({
Expand Down
33 changes: 20 additions & 13 deletions packages/router/src/router_config_loader.ts
Expand Up @@ -6,9 +6,9 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Compiler, InjectFlags, InjectionToken, Injector, NgModuleFactory} from '@angular/core';
import {Compiler, Injectable, InjectFlags, InjectionToken, Injector, NgModuleFactory} from '@angular/core';
import {ConnectableObservable, from, Observable, of, Subject} from 'rxjs';
import {catchError, map, mergeMap, refCount, tap} from 'rxjs/operators';
import {catchError, finalize, map, mergeMap, refCount, tap} from 'rxjs/operators';

import {LoadChildren, LoadedRouterConfig, Route, Routes} from './models';
import {flatten, wrapIntoObservable} from './utils/collection';
Expand All @@ -29,15 +29,22 @@ const NG_DEV_MODE = typeof ngDevMode === 'undefined' || !!ngDevMode;
*/
export const ROUTES = new InjectionToken<Route[][]>('ROUTES');

@Injectable()
export class RouterConfigLoader {
private routeLoaders = new WeakMap<Route, Observable<LoadedRouterConfig>>();
onLoadStartListener?: (r: Route) => void;
onLoadEndListener?: (r: Route) => void;

constructor(
private injector: Injector, private compiler: Compiler,
private onLoadStartListener?: (r: Route) => void,
private onLoadEndListener?: (r: Route) => void) {}
private injector: Injector,
private compiler: Compiler,
) {}

load(parentInjector: Injector, route: Route): Observable<LoadedRouterConfig> {
if (route._loader$) {
return route._loader$;
if (this.routeLoaders.get(route)) {
return this.routeLoaders.get(route)!;
} else if (route._loadedRoutes) {
return of({routes: route._loadedRoutes, injector: route._loadedInjector});
}

if (this.onLoadStartListener) {
Expand All @@ -60,15 +67,15 @@ export class RouterConfigLoader {
NG_DEV_MODE && validateConfig(routes);
return {routes, injector};
}),
catchError((err) => {
route._loader$ = undefined;
throw err;
finalize(() => {
this.routeLoaders.delete(route);
}),
);
// Use custom ConnectableObservable as share in runners pipe increasing the bundle size too much
route._loader$ = new ConnectableObservable(loadRunner, () => new Subject<LoadedRouterConfig>())
.pipe(refCount());
return route._loader$;
const loader = new ConnectableObservable(loadRunner, () => new Subject<LoadedRouterConfig>())
.pipe(refCount());
this.routeLoaders.set(route, loader);
return loader;
}

private loadModuleFactory(loadChildren: LoadChildren): Observable<NgModuleFactory<any>> {
Expand Down
3 changes: 2 additions & 1 deletion packages/router/src/router_module.ts
Expand Up @@ -20,7 +20,7 @@ import {Route, Routes} from './models';
import {DefaultTitleStrategy, TitleStrategy} from './page_title_strategy';
import {RouteReuseStrategy} from './route_reuse_strategy';
import {ErrorHandler, Router} from './router';
import {ROUTES} from './router_config_loader';
import {RouterConfigLoader, ROUTES} from './router_config_loader';
import {ChildrenOutletContexts} from './router_outlet_context';
import {NoPreloading, PreloadAllModules, PreloadingStrategy, RouterPreloader} from './router_preloader';
import {RouterScroller} from './router_scroller';
Expand Down Expand Up @@ -65,6 +65,7 @@ export const ROUTER_PROVIDERS: Provider[] = [
NoPreloading,
PreloadAllModules,
{provide: ROUTER_CONFIGURATION, useValue: {enableTracing: false}},
RouterConfigLoader,
];

export function routerNgProbeToken() {
Expand Down
8 changes: 1 addition & 7 deletions packages/router/src/router_preloader.ts
Expand Up @@ -73,17 +73,11 @@ export class NoPreloading implements PreloadingStrategy {
*/
@Injectable()
export class RouterPreloader implements OnDestroy {
private loader: RouterConfigLoader;
private subscription?: Subscription;

constructor(
private router: Router, compiler: Compiler, private injector: Injector,
private preloadingStrategy: PreloadingStrategy) {
const onStartLoad = (r: Route) => router.triggerEvent(new RouteConfigLoadStart(r));
const onEndLoad = (r: Route) => router.triggerEvent(new RouteConfigLoadEnd(r));

this.loader = new RouterConfigLoader(injector, compiler, onStartLoad, onEndLoad);
}
private preloadingStrategy: PreloadingStrategy, private loader: RouterConfigLoader) {}

setUpPreloading(): void {
this.subscription =
Expand Down

0 comments on commit ea8256f

Please sign in to comment.