refactor(router): Only create the Route injector when the path matches (#45990)

Currently, the `Router` will create the `EnvironmentInjector` for a
`Route` with `providers` as soon as it _attempts_ to match segments to
the `Route`. Instead, this change updates the logic to only create the
injector once we have confirmed that the `Route` matches. Note that this
will include partial matches where the final navigation still fails with
a "cannot match any routes to segment" error. We need to maintain the
injector hierarchy so we still need to create parent injectors before
descending into the child routes and determining if there is a full
match or just a partial one.

resolves #45988

PR Close #45990
This commit is contained in:
Andrew Scott
2022-05-13 09:53:55 -07:00
committed by Jessica Janiuk
parent 9c1735bdde
commit 4d2e006372
4 changed files with 69 additions and 15 deletions
@@ -1247,6 +1247,9 @@
{
"name": "getOrCreateNodeInjectorForNode"
},
{
"name": "getOrCreateRouteInjectorIfNeeded"
},
{
"name": "getOrCreateTComponentView"
},
+8 -13
View File
@@ -6,8 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/
import {createEnvironmentInjector, EnvironmentInjector} from '@angular/core';
import {EmptyError, from, Observable, Observer, of, throwError} from 'rxjs';
import {EnvironmentInjector} from '@angular/core';
import {EmptyError, from, Observable, of, throwError} from 'rxjs';
import {catchError, concatMap, first, last, map, mergeMap, scan, tap} from 'rxjs/operators';
import {CanLoadFn, LoadedRouterConfig, Route, Routes} from './models';
@@ -16,7 +16,7 @@ import {RouterConfigLoader} from './router_config_loader';
import {navigationCancelingError, Params, PRIMARY_OUTLET} from './shared';
import {UrlSegment, UrlSegmentGroup, UrlSerializer, UrlTree} from './url_tree';
import {forEach, wrapIntoObservable} from './utils/collection';
import {getOutlet, sortByMatchingOutlets} from './utils/config';
import {getOrCreateRouteInjectorIfNeeded, getOutlet, sortByMatchingOutlets} from './utils/config';
import {isImmediateMatch, match, noLeftoversInUrl, split} from './utils/config_matching';
import {isCanLoad, isFunction, isUrlTree} from './utils/type_guards';
@@ -183,17 +183,8 @@ class ApplyRedirects {
allowRedirects: boolean): Observable<UrlSegmentGroup> {
return from(routes).pipe(
concatMap(r => {
if (r.providers && !r._injector) {
r._injector = createEnvironmentInjector(r.providers, injector, `Route: ${r.path}`);
}
// We specifically _do not_ want to include the _loadedInjector here. The loaded injector
// only applies to the route's children, not the route itself. Note that this distinction
// only applies here to any tokens we try to retrieve during this phase. At the moment,
// that only includes `canLoad`, which won't run again once the child module is loaded. As
// a result, this makes no difference right now, but could in the future if there are more
// actions here that need DI (for example, a canMatch guard).
const expanded$ = this.expandSegmentAgainstRoute(
r._injector ?? injector, segmentGroup, routes, r, segments, outlet, allowRedirects);
injector, segmentGroup, routes, r, segments, outlet, allowRedirects);
return expanded$.pipe(catchError((e: any) => {
if (e instanceof NoMatch) {
return of(null);
@@ -280,6 +271,8 @@ class ApplyRedirects {
injector: EnvironmentInjector, rawSegmentGroup: UrlSegmentGroup, route: Route,
segments: UrlSegment[], outlet: string): Observable<UrlSegmentGroup> {
if (route.path === '**') {
// Only create the Route's `EnvironmentInjector` if it matches the attempted navigation
injector = getOrCreateRouteInjectorIfNeeded(route, injector);
if (route.loadChildren) {
const loaded$ = route._loadedRoutes ?
of({routes: route._loadedRoutes, injector: route._loadedInjector}) :
@@ -297,6 +290,8 @@ class ApplyRedirects {
const {matched, consumedSegments, remainingSegments} = match(rawSegmentGroup, route, segments);
if (!matched) return noMatch(rawSegmentGroup);
// Only create the Route's `EnvironmentInjector` if it matches the attempted navigation
injector = getOrCreateRouteInjectorIfNeeded(route, injector);
const childConfig$ = this.getChildConfig(injector, route, segments);
return childConfig$.pipe(mergeMap((routerConfig: LoadedRouterConfig) => {
+18 -1
View File
@@ -6,13 +6,30 @@
* found in the LICENSE file at https://angular.io/license
*/
import {EnvironmentInjector, Type, ɵisStandalone as isStandalone} from '@angular/core';
import {createEnvironmentInjector, EnvironmentInjector, Type, ɵisStandalone as isStandalone} from '@angular/core';
import {EmptyOutletComponent} from '../components/empty_outlet';
import {Route, Routes} from '../models';
import {ActivatedRouteSnapshot} from '../router_state';
import {PRIMARY_OUTLET} from '../shared';
/**
* Creates an `EnvironmentInjector` if the `Route` has providers and one does not already exist
* and returns the injector. Otherwise, if the `Route` does not have `providers`, returns the
* `currentInjector`.
*
* @param route The route that might have providers
* @param currentInjector The parent injector of the `Route`
*/
export function getOrCreateRouteInjectorIfNeeded(
route: Route, currentInjector: EnvironmentInjector) {
if (route.providers && !route._injector) {
route._injector =
createEnvironmentInjector(route.providers, currentInjector, `Route: ${route.path}`);
}
return route._injector ?? currentInjector;
}
export function getLoadedRoutes(route: Route): Route[]|undefined {
return route._loadedRoutes;
}
+40 -1
View File
@@ -15,7 +15,7 @@ import {applyRedirects} from '../src/apply_redirects';
import {Route, Routes} from '../src/models';
import {RouterConfigLoader} from '../src/router_config_loader';
import {DefaultUrlSerializer, equalSegments, UrlSegment, UrlSegmentGroup, UrlTree} from '../src/url_tree';
import {getLoadedRoutes} from '../src/utils/config';
import {getLoadedRoutes, getProvidersInjector} from '../src/utils/config';
describe('applyRedirects', () => {
const serializer = new DefaultUrlSerializer();
@@ -190,6 +190,45 @@ describe('applyRedirects', () => {
});
});
it('should not create injector for Route if the route does not match', () => {
const routes = [
{path: '', pathMatch: 'full' as const, providers: []},
{
path: 'a',
component: ComponentA,
children: [
{path: 'b', component: ComponentB},
],
},
];
checkRedirect(routes, '/a/b', (t: UrlTree) => {
expectTreeToBe(t, '/a/b');
expect(getProvidersInjector(routes[0])).not.toBeDefined();
});
});
it('should create injectors for partial Route route matches', () => {
const routes = [
{
path: 'a',
component: ComponentA,
providers: [],
},
{path: 'doesNotMatch', providers: []},
];
applyRedirects(testModule.injector, null!, serializer, tree('a/b/c'), routes).subscribe({
next: () => {
throw 'Should not be reached';
},
error: () => {
// The 'a' segment matched, so we needed to create the injector for the `Route`
expect(getProvidersInjector(routes[0])).toBeDefined();
// The second `Route` did not match at all so we should not create an injector for it
expect(getProvidersInjector(routes[1])).not.toBeDefined();
}
});
});
describe('lazy loading', () => {
it('should load config on demand', () => {
const loadedConfig = {