fix(router): Routed components never inherit RouterOutlet EnvironmentInjector (#54265)

This commit ensures components in the route config predictably always
get their providers from the hierarchy available to routes rather than
sometimes being dependent on where they are inserted.

fixes #53369

BREAKING CHANGE: Providers available to the routed components always
come from the injector heirarchy of the routes and never inherit from
the `RouterOutlet`. This means that providers available only to the
component that defines the `RouterOutlet` will no longer be available to
route components in any circumstances. This was already the case
whenever routes defined providers, either through lazy loading an
`NgModule` or through explicit `providers` on the route config.

PR Close #54265
This commit is contained in:
Andrew Scott 2024-02-05 10:30:55 -08:00
parent 87f3f27f90
commit 3839cfbb18
7 changed files with 107 additions and 51 deletions

View file

@ -183,6 +183,7 @@ export class ChildActivationStart {
// @public
export class ChildrenOutletContexts {
constructor(parentInjector: EnvironmentInjector);
// (undocumented)
getContext(childName: string): OutletContext | null;
// (undocumented)
@ -539,12 +540,13 @@ export type OnSameUrlNavigation = 'reload' | 'ignore';
// @public
export class OutletContext {
constructor(injector: EnvironmentInjector);
// (undocumented)
attachRef: ComponentRef<any> | null;
// (undocumented)
children: ChildrenOutletContexts;
// (undocumented)
injector: EnvironmentInjector | null;
injector: EnvironmentInjector;
// (undocumented)
outlet: RouterOutletContract | null;
// (undocumented)
@ -882,7 +884,7 @@ export class RouterOutlet implements OnDestroy, OnInit, RouterOutletContract {
// (undocumented)
activateEvents: EventEmitter<any>;
// (undocumented)
activateWith(activatedRoute: ActivatedRoute, environmentInjector?: EnvironmentInjector | null): void;
activateWith(activatedRoute: ActivatedRoute, environmentInjector: EnvironmentInjector): void;
attach(ref: ComponentRef<any>, activatedRoute: ActivatedRoute): void;
attachEvents: EventEmitter<unknown>;
// (undocumented)
@ -915,7 +917,7 @@ export interface RouterOutletContract {
activatedRoute: ActivatedRoute | null;
activatedRouteData: Data;
activateEvents?: EventEmitter<unknown>;
activateWith(activatedRoute: ActivatedRoute, environmentInjector: EnvironmentInjector | null): void;
activateWith(activatedRoute: ActivatedRoute, environmentInjector: EnvironmentInjector): void;
attach(ref: ComponentRef<unknown>, activatedRoute: ActivatedRoute): void;
attachEvents?: EventEmitter<unknown>;
component: Object | null;

View file

@ -354,17 +354,14 @@ describe('getInjectorMetadata', () => {
expect(injectorMetadata[4]).toBeDefined();
expect(injectorMetadata[5]).toBeDefined();
expect(injectorMetadata[6]).toBeDefined();
expect(injectorMetadata[7]).toBeDefined();
expect(injectorMetadata[0]!.source).toBe(lazyComponent.elementRef.nativeElement);
expect(injectorMetadata[1]!.source)
.toBe(myStandaloneComponent.routerOutlet!.nativeElement);
expect(injectorMetadata[2]!.source).toBe(myStandaloneComponent.elementRef.nativeElement);
expect(injectorMetadata[3]!.source).toBe('Standalone[LazyComponent]');
expect(injectorMetadata[4]!.source).toBe('Standalone[MyStandaloneComponent]');
expect(injectorMetadata[5]!.source).toBe('DynamicTestModule');
expect(injectorMetadata[6]!.source).toBe('Platform: core');
expect(injectorMetadata[7]!.source).toBeNull();
expect(injectorMetadata[4]!.source).toBe('DynamicTestModule');
expect(injectorMetadata[5]!.source).toBe('Platform: core');
expect(injectorMetadata[0]!.type).toBe('element');
expect(injectorMetadata[1]!.type).toBe('element');
@ -372,8 +369,6 @@ describe('getInjectorMetadata', () => {
expect(injectorMetadata[3]!.type).toBe('environment');
expect(injectorMetadata[4]!.type).toBe('environment');
expect(injectorMetadata[5]!.type).toBe('environment');
expect(injectorMetadata[6]!.type).toBe('environment');
expect(injectorMetadata[7]!.type).toBe('null');
}
}));
@ -932,10 +927,10 @@ describe('getDependenciesFromInjectable', () => {
standalone: true
})
class MyStandaloneComponentB {
myService = inject(MyService);
myService = inject(MyService, {optional: true});
myServiceB = inject(MyServiceB, {optional: true});
myServiceC = inject(MyServiceC, {skipSelf: true});
myInjectionTokenValue = inject(myInjectionToken);
myServiceC = inject(MyServiceC, {skipSelf: true, optional: true});
myInjectionTokenValue = inject(myInjectionToken, {optional: true});
injector = inject(Injector, {self: true, host: true});
myServiceD = inject(MyServiceD);
myServiceG = inject(MyServiceG);
@ -995,7 +990,7 @@ describe('getDependenciesFromInjectable', () => {
expect(parentComponentDep.token).toBe(MyStandaloneComponent);
expect(dependenciesOfMyStandaloneComponentB[0].flags).toEqual({
optional: false,
optional: true,
skipSelf: false,
self: false,
host: false,
@ -1007,13 +1002,13 @@ describe('getDependenciesFromInjectable', () => {
host: false,
});
expect(myServiceCDep.flags).toEqual({
optional: false,
optional: true,
skipSelf: true,
self: false,
host: false,
});
expect(myInjectionTokenValueDep.flags).toEqual({
optional: false,
optional: true,
skipSelf: false,
self: false,
host: false,
@ -1045,18 +1040,18 @@ describe('getDependenciesFromInjectable', () => {
expect(dependenciesOfMyStandaloneComponentB[0].value).toBe(myStandalonecomponentB.myService);
expect(myServiceBDep.value).toBe('hello world');
expect(myServiceCDep.value).toBe(123);
expect(myInjectionTokenValueDep.value).toBe(myServiceCInstance);
expect(myServiceBDep.value).toBe(null);
expect(myServiceCDep.value).toBe(null);
expect(myInjectionTokenValueDep.value).toBe(null);
expect(injectorDep.value).toBe(myStandalonecomponentB.injector);
expect(myServiceDDep.value).toBe('123');
expect(myServiceGDep.value).toBe(myStandalonecomponentB.myServiceG);
expect(parentComponentDep.value).toBe(myStandalonecomponentB.parentComponent);
expect(dependenciesOfMyStandaloneComponentB[0].providedIn).toBe(standaloneInjector);
expect(myServiceBDep.providedIn).toBe(standaloneInjector);
expect(myServiceCDep.providedIn).toBe(standaloneInjector);
expect(myInjectionTokenValueDep.providedIn).toBe(standaloneInjector);
expect(dependenciesOfMyStandaloneComponentB[0].providedIn).toBe(undefined);
expect(myServiceBDep.providedIn).toBe(undefined);
expect(myServiceCDep.providedIn).toBe(undefined);
expect(myInjectionTokenValueDep.providedIn).toBe(undefined);
expect(injectorDep.providedIn).toBe(myStandalonecomponentB.injector);
expect(myServiceDDep.providedIn).toBe(standaloneInjector.get(Injector, null, {
skipSelf: true
@ -1266,13 +1261,12 @@ describe('getInjectorResolutionPath', () => {
* NodeInjector[RouterOutlet],
* NodeInjector[MyStandaloneComponent],
* R3Injector[LazyComponent],
* R3Injector[MyStandaloneComponent],
* R3Injector[Root],
* R3Injector[Platform],
* NullInjector
* ]
*/
expect(path.length).toBe(8);
expect(path.length).toBe(7);
expect(path[0]).toBe(lazyComponentNodeInjector);
@ -1291,16 +1285,13 @@ describe('getInjectorResolutionPath', () => {
expect(path[4]).toBeInstanceOf(R3Injector);
expect((path[4] as R3Injector).scopes.has('environment')).toBeTrue();
expect((path[4] as R3Injector).source).toBe('Standalone[MyStandaloneComponent]');
expect((path[4] as R3Injector).source).toBe('DynamicTestModule');
expect((path[4] as R3Injector).scopes.has('root')).toBeTrue();
expect(path[5]).toBeInstanceOf(R3Injector);
expect((path[5] as R3Injector).scopes.has('environment')).toBeTrue();
expect((path[5] as R3Injector).scopes.has('root')).toBeTrue();
expect((path[5] as R3Injector).scopes.has('platform')).toBeTrue();
expect(path[6]).toBeInstanceOf(R3Injector);
expect((path[6] as R3Injector).scopes.has('platform')).toBeTrue();
expect(path[7]).toBeInstanceOf(NullInjector);
expect(path[6]).toBeInstanceOf(NullInjector);
}
}));
});

View file

@ -71,10 +71,7 @@ export interface RouterOutletContract {
/**
* Called by the `Router` when the outlet should activate (create a component).
*/
activateWith(
activatedRoute: ActivatedRoute,
environmentInjector: EnvironmentInjector | null,
): void;
activateWith(activatedRoute: ActivatedRoute, environmentInjector: EnvironmentInjector): void;
/**
* A request to destroy the currently activated component.
@ -216,7 +213,6 @@ export class RouterOutlet implements OnDestroy, OnInit, RouterOutletContract {
private parentContexts = inject(ChildrenOutletContexts);
private location = inject(ViewContainerRef);
private changeDetector = inject(ChangeDetectorRef);
private environmentInjector = inject(EnvironmentInjector);
private inputBinder = inject(INPUT_BINDER, {optional: true});
/** @nodoc */
readonly supportsBindingToComponentInputs = true;
@ -350,7 +346,7 @@ export class RouterOutlet implements OnDestroy, OnInit, RouterOutletContract {
}
}
activateWith(activatedRoute: ActivatedRoute, environmentInjector?: EnvironmentInjector | null) {
activateWith(activatedRoute: ActivatedRoute, environmentInjector: EnvironmentInjector) {
if (this.isActivated) {
throw new RuntimeError(
RuntimeErrorCode.OUTLET_ALREADY_ACTIVATED,
@ -368,7 +364,7 @@ export class RouterOutlet implements OnDestroy, OnInit, RouterOutletContract {
this.activated = location.createComponent(component, {
index: location.length,
injector,
environmentInjector: environmentInjector ?? this.environmentInjector,
environmentInjector: environmentInjector,
});
// Calling `markForCheck` to make sure we will run the change detection when the
// `RouterOutlet` is inside a `ChangeDetectionStrategy.OnPush` component.

View file

@ -224,7 +224,7 @@ export class ActivateRoutes {
const injector = getClosestRouteInjector(future.snapshot);
context.attachRef = null;
context.route = future;
context.injector = injector;
context.injector = injector ?? context.injector;
if (context.outlet) {
// Activate the outlet when it has already been instantiated
// Otherwise it will get activated from its `ngOnInit` when instantiated

View file

@ -19,9 +19,9 @@ import {ActivatedRoute} from './router_state';
export class OutletContext {
outlet: RouterOutletContract | null = null;
route: ActivatedRoute | null = null;
injector: EnvironmentInjector | null = null;
children = new ChildrenOutletContexts();
children = new ChildrenOutletContexts(this.injector);
attachRef: ComponentRef<any> | null = null;
constructor(public injector: EnvironmentInjector) {}
}
/**
@ -34,6 +34,9 @@ export class ChildrenOutletContexts {
// contexts for child outlets, by name.
private contexts = new Map<string, OutletContext>();
/** @nodoc */
constructor(private parentInjector: EnvironmentInjector) {}
/** Called when a `RouterOutlet` directive is instantiated */
onChildOutletCreated(childName: string, outlet: RouterOutletContract): void {
const context = this.getOrCreateContext(childName);
@ -72,7 +75,7 @@ export class ChildrenOutletContexts {
let context = this.getContext(childName);
if (!context) {
context = new OutletContext();
context = new OutletContext(this.parentInjector);
this.contexts.set(childName, context);
}

View file

@ -7,7 +7,16 @@
*/
import {CommonModule, NgForOf} from '@angular/common';
import {Component, Input, Type} from '@angular/core';
import {
Component,
EnvironmentInjector,
Input,
NgModule,
Type,
createEnvironmentInjector,
importProvidersFrom,
inject,
} from '@angular/core';
import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
import {
provideRouter,
@ -15,8 +24,9 @@ import {
RouterModule,
RouterOutlet,
withComponentInputBinding,
} from '@angular/router/src';
} from '@angular/router';
import {RouterTestingHarness} from '@angular/router/testing';
import {InjectionToken} from '../../../core/src/di';
describe('router outlet name', () => {
it('should support name binding', fakeAsync(() => {
@ -381,6 +391,44 @@ describe('component input binding', () => {
});
});
describe('injectors', () => {
it('should always use environment injector from route hierarchy and not inherit from outlet', async () => {
let childTokenValue: any = null;
const TOKEN = new InjectionToken<any>('');
@Component({
template: '',
standalone: true,
})
class Child {
constructor() {
childTokenValue = inject(TOKEN as any, {optional: true});
}
}
@NgModule({
providers: [{provide: TOKEN, useValue: 'some value'}],
})
class ModWithProviders {}
@Component({
template: '<router-outlet/>',
imports: [RouterOutlet, ModWithProviders],
standalone: true,
})
class App {}
TestBed.configureTestingModule({
providers: [provideRouter([{path: 'a', component: Child}])],
});
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
await TestBed.inject(Router).navigateByUrl('/a');
fixture.detectChanges();
expect(childTokenValue).toEqual(null);
});
});
function advance(fixture: ComponentFixture<unknown>, millis?: number): void {
tick(millis);
fixture.detectChanges();

View file

@ -186,7 +186,11 @@ describe('Router', () => {
// Since we only test the guards, we don't need to provide a full navigation
// transition object with all properties set.
const testTransition = {
guards: getAllRouteGuards(futureState, empty, new ChildrenOutletContexts()),
guards: getAllRouteGuards(
futureState,
empty,
new ChildrenOutletContexts(TestBed.inject(EnvironmentInjector)),
),
} as NavigationTransition;
of(testTransition)
@ -242,7 +246,11 @@ describe('Router', () => {
// Since we only test the guards, we don't need to provide a full navigation
// transition object with all properties set.
const testTransition = {
guards: getAllRouteGuards(futureState, empty, new ChildrenOutletContexts()),
guards: getAllRouteGuards(
futureState,
empty,
new ChildrenOutletContexts(TestBed.inject(EnvironmentInjector)),
),
} as NavigationTransition;
of(testTransition)
@ -296,7 +304,11 @@ describe('Router', () => {
// Since we only test the guards, we don't need to provide a full navigation
// transition object with all properties set.
const testTransition = {
guards: getAllRouteGuards(futureState, currentState, new ChildrenOutletContexts()),
guards: getAllRouteGuards(
futureState,
currentState,
new ChildrenOutletContexts(TestBed.inject(EnvironmentInjector)),
),
} as NavigationTransition;
of(testTransition)
@ -368,7 +380,11 @@ describe('Router', () => {
// Since we only test the guards, we don't need to provide a full navigation
// transition object with all properties set.
const testTransition = {
guards: getAllRouteGuards(futureState, currentState, new ChildrenOutletContexts()),
guards: getAllRouteGuards(
futureState,
currentState,
new ChildrenOutletContexts(TestBed.inject(EnvironmentInjector)),
),
} as NavigationTransition;
of(testTransition)
@ -841,7 +857,7 @@ function checkResolveData(
// Since we only test the guards and their resolve data function, we don't need to provide
// a full navigation transition object with all properties set.
of({
guards: getAllRouteGuards(future, curr, new ChildrenOutletContexts()),
guards: getAllRouteGuards(future, curr, new ChildrenOutletContexts(injector)),
} as NavigationTransition)
.pipe(resolveDataOperator('emptyOnly', injector))
.subscribe(check, (e) => {
@ -858,7 +874,7 @@ function checkGuards(
// Since we only test the guards, we don't need to provide a full navigation
// transition object with all properties set.
of({
guards: getAllRouteGuards(future, curr, new ChildrenOutletContexts()),
guards: getAllRouteGuards(future, curr, new ChildrenOutletContexts(injector)),
} as NavigationTransition)
.pipe(checkGuardsOperator(injector))
.subscribe({