From ce20ed067f12714ee6488ffe07f1a1c18bb60a15 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 16 Jun 2022 09:47:11 -0700 Subject: [PATCH] fix(router): Ensure Route injector is created before running CanMatch guards (#46394) Once a `Route` matches via the `match` or `path` property, we need to immediately create the injector for the route (if it has providers) before running the `CanMatch` guards. This is necessary because the `CanMatch` guards might be provided in the `Route` providers. Fixes #46386 PR Close #46394 --- packages/router/src/apply_redirects.ts | 7 ++-- packages/router/src/recognize.ts | 7 ++-- packages/router/src/utils/config_matching.ts | 12 ++++--- packages/router/test/apply_redirects.spec.ts | 37 +++++++++++++++++++- 4 files changed, 50 insertions(+), 13 deletions(-) diff --git a/packages/router/src/apply_redirects.ts b/packages/router/src/apply_redirects.ts index 7564706352b..bbc8cc95314 100644 --- a/packages/router/src/apply_redirects.ts +++ b/packages/router/src/apply_redirects.ts @@ -15,7 +15,7 @@ import {runCanLoadGuards} from './operators/check_guards'; import {RouterConfigLoader} from './router_config_loader'; import {navigationCancelingError, Params, PRIMARY_OUTLET} from './shared'; import {createRoot, squashSegmentGroup, UrlSegment, UrlSegmentGroup, UrlSerializer, UrlTree} from './url_tree'; -import {forEach, wrapIntoObservable} from './utils/collection'; +import {forEach} from './utils/collection'; import {getOrCreateRouteInjectorIfNeeded, getOutlet, sortByMatchingOutlets} from './utils/config'; import {isImmediateMatch, match, matchWithChecks, noLeftoversInUrl, split} from './utils/config_matching'; @@ -289,9 +289,8 @@ class ApplyRedirects { switchMap(({matched, consumedSegments, remainingSegments}) => { if (!matched) return noMatch(rawSegmentGroup); - // Only create the Route's `EnvironmentInjector` if it matches the attempted - // navigation - injector = getOrCreateRouteInjectorIfNeeded(route, injector); + // If the route has an injector created from providers, we should start using that. + injector = route._injector ?? injector; const childConfig$ = this.getChildConfig(injector, route, segments); return childConfig$.pipe(mergeMap((routerConfig: LoadedRouterConfig) => { diff --git a/packages/router/src/recognize.ts b/packages/router/src/recognize.ts index 65c4579f35a..40721f24e2f 100644 --- a/packages/router/src/recognize.ts +++ b/packages/router/src/recognize.ts @@ -173,7 +173,7 @@ export class Recognizer { let matchResult: Observable<{ snapshot: ActivatedRouteSnapshot, consumedSegments: UrlSegment[], - remainingSegments: UrlSegment[] + remainingSegments: UrlSegment[], }|null>; if (route.path === '**') { @@ -219,9 +219,8 @@ export class Recognizer { return of(null); } const {snapshot, consumedSegments, remainingSegments} = result; - // Only create the Route's `EnvironmentInjector` if it matches the attempted - // navigation - injector = getOrCreateRouteInjectorIfNeeded(route, injector); + // If the route has an injector created from providers, we should start using that. + injector = route._injector ?? injector; const childInjector = route._loadedInjector ?? injector; const childConfig: Route[] = getChildConfig(route); diff --git a/packages/router/src/utils/config_matching.ts b/packages/router/src/utils/config_matching.ts index deea2b6a417..fa55dbcb234 100644 --- a/packages/router/src/utils/config_matching.ts +++ b/packages/router/src/utils/config_matching.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Injector} from '@angular/core'; +import {EnvironmentInjector, Injector} from '@angular/core'; import {Observable, of} from 'rxjs'; import {map} from 'rxjs/operators'; @@ -16,7 +16,7 @@ import {defaultUrlMatcher, PRIMARY_OUTLET} from '../shared'; import {UrlSegment, UrlSegmentGroup, UrlSerializer} from '../url_tree'; import {forEach} from './collection'; -import {getOutlet} from './config'; +import {getOrCreateRouteInjectorIfNeeded, getOutlet} from './config'; export interface MatchResult { matched: boolean; @@ -35,12 +35,16 @@ const noMatch: MatchResult = { }; export function matchWithChecks( - segmentGroup: UrlSegmentGroup, route: Route, segments: UrlSegment[], injector: Injector, - urlSerializer: UrlSerializer): Observable { + segmentGroup: UrlSegmentGroup, route: Route, segments: UrlSegment[], + injector: EnvironmentInjector, urlSerializer: UrlSerializer): Observable { const result = match(segmentGroup, route, segments); if (!result.matched) { return of(result); } + + // Only create the Route's `EnvironmentInjector` if it matches the attempted + // navigation + injector = getOrCreateRouteInjectorIfNeeded(route, injector); return runCanMatchGuards(injector, route, segments, urlSerializer) .pipe( map((v) => v === true ? result : {...noMatch}), diff --git a/packages/router/test/apply_redirects.spec.ts b/packages/router/test/apply_redirects.spec.ts index a6d7d16eb9f..3542f2f3f87 100644 --- a/packages/router/test/apply_redirects.spec.ts +++ b/packages/router/test/apply_redirects.spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {EnvironmentInjector, NgModuleRef} from '@angular/core'; +import {EnvironmentInjector, Injectable, NgModuleRef} from '@angular/core'; import {fakeAsync, TestBed, tick} from '@angular/core/testing'; import {Observable, of} from 'rxjs'; import {delay, tap} from 'rxjs/operators'; @@ -229,6 +229,41 @@ describe('applyRedirects', () => { }); }); + it('should support CanMatch providers on the route', () => { + @Injectable({providedIn: 'root'}) + class CanMatchGuard { + canMatch() { + return true; + } + } + + const routes = [ + { + path: 'a', + component: ComponentA, + canMatch: [CanMatchGuard], + providers: [CanMatchGuard], + }, + { + path: 'a', + component: ComponentA, + providers: [], + } + ]; + applyRedirects(testModule.injector, null!, serializer, tree('a'), routes).subscribe({ + next: () => { + // 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 because the first did so we should not create an + // injector for it + expect(getProvidersInjector(routes[1])).not.toBeDefined(); + }, + error: () => { + throw 'Should not be reached'; + } + }); + }) + describe('lazy loading', () => { it('should load config on demand', () => { const loadedConfig = {