Skip to content

Commit 3839cfb

Browse files
atscottatscott
committed
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
1 parent 87f3f27 commit 3839cfb

File tree

7 files changed

+107
-51
lines changed

7 files changed

+107
-51
lines changed

‎goldens/public-api/router/index.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ export class ChildActivationStart {
183183

184184
// @public
185185
export class ChildrenOutletContexts {
186+
constructor(parentInjector: EnvironmentInjector);
186187
// (undocumented)
187188
getContext(childName: string): OutletContext | null;
188189
// (undocumented)
@@ -539,12 +540,13 @@ export type OnSameUrlNavigation = 'reload' | 'ignore';
539540

540541
// @public
541542
export class OutletContext {
543+
constructor(injector: EnvironmentInjector);
542544
// (undocumented)
543545
attachRef: ComponentRef<any> | null;
544546
// (undocumented)
545547
children: ChildrenOutletContexts;
546548
// (undocumented)
547-
injector: EnvironmentInjector | null;
549+
injector: EnvironmentInjector;
548550
// (undocumented)
549551
outlet: RouterOutletContract | null;
550552
// (undocumented)
@@ -882,7 +884,7 @@ export class RouterOutlet implements OnDestroy, OnInit, RouterOutletContract {
882884
// (undocumented)
883885
activateEvents: EventEmitter<any>;
884886
// (undocumented)
885-
activateWith(activatedRoute: ActivatedRoute, environmentInjector?: EnvironmentInjector | null): void;
887+
activateWith(activatedRoute: ActivatedRoute, environmentInjector: EnvironmentInjector): void;
886888
attach(ref: ComponentRef<any>, activatedRoute: ActivatedRoute): void;
887889
attachEvents: EventEmitter<unknown>;
888890
// (undocumented)
@@ -915,7 +917,7 @@ export interface RouterOutletContract {
915917
activatedRoute: ActivatedRoute | null;
916918
activatedRouteData: Data;
917919
activateEvents?: EventEmitter<unknown>;
918-
activateWith(activatedRoute: ActivatedRoute, environmentInjector: EnvironmentInjector | null): void;
920+
activateWith(activatedRoute: ActivatedRoute, environmentInjector: EnvironmentInjector): void;
919921
attach(ref: ComponentRef<unknown>, activatedRoute: ActivatedRoute): void;
920922
attachEvents?: EventEmitter<unknown>;
921923
component: Object | null;

‎packages/core/test/acceptance/injector_profiler_spec.ts‎

Lines changed: 20 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -354,26 +354,21 @@ describe('getInjectorMetadata', () => {
354354
expect(injectorMetadata[4]).toBeDefined();
355355
expect(injectorMetadata[5]).toBeDefined();
356356
expect(injectorMetadata[6]).toBeDefined();
357-
expect(injectorMetadata[7]).toBeDefined();
358357

359358
expect(injectorMetadata[0]!.source).toBe(lazyComponent.elementRef.nativeElement);
360359
expect(injectorMetadata[1]!.source)
361360
.toBe(myStandaloneComponent.routerOutlet!.nativeElement);
362361
expect(injectorMetadata[2]!.source).toBe(myStandaloneComponent.elementRef.nativeElement);
363362
expect(injectorMetadata[3]!.source).toBe('Standalone[LazyComponent]');
364-
expect(injectorMetadata[4]!.source).toBe('Standalone[MyStandaloneComponent]');
365-
expect(injectorMetadata[5]!.source).toBe('DynamicTestModule');
366-
expect(injectorMetadata[6]!.source).toBe('Platform: core');
367-
expect(injectorMetadata[7]!.source).toBeNull();
363+
expect(injectorMetadata[4]!.source).toBe('DynamicTestModule');
364+
expect(injectorMetadata[5]!.source).toBe('Platform: core');
368365

369366
expect(injectorMetadata[0]!.type).toBe('element');
370367
expect(injectorMetadata[1]!.type).toBe('element');
371368
expect(injectorMetadata[2]!.type).toBe('element');
372369
expect(injectorMetadata[3]!.type).toBe('environment');
373370
expect(injectorMetadata[4]!.type).toBe('environment');
374371
expect(injectorMetadata[5]!.type).toBe('environment');
375-
expect(injectorMetadata[6]!.type).toBe('environment');
376-
expect(injectorMetadata[7]!.type).toBe('null');
377372
}
378373
}));
379374

@@ -932,10 +927,10 @@ describe('getDependenciesFromInjectable', () => {
932927
standalone: true
933928
})
934929
class MyStandaloneComponentB {
935-
myService = inject(MyService);
930+
myService = inject(MyService, {optional: true});
936931
myServiceB = inject(MyServiceB, {optional: true});
937-
myServiceC = inject(MyServiceC, {skipSelf: true});
938-
myInjectionTokenValue = inject(myInjectionToken);
932+
myServiceC = inject(MyServiceC, {skipSelf: true, optional: true});
933+
myInjectionTokenValue = inject(myInjectionToken, {optional: true});
939934
injector = inject(Injector, {self: true, host: true});
940935
myServiceD = inject(MyServiceD);
941936
myServiceG = inject(MyServiceG);
@@ -995,7 +990,7 @@ describe('getDependenciesFromInjectable', () => {
995990
expect(parentComponentDep.token).toBe(MyStandaloneComponent);
996991

997992
expect(dependenciesOfMyStandaloneComponentB[0].flags).toEqual({
998-
optional: false,
993+
optional: true,
999994
skipSelf: false,
1000995
self: false,
1001996
host: false,
@@ -1007,13 +1002,13 @@ describe('getDependenciesFromInjectable', () => {
10071002
host: false,
10081003
});
10091004
expect(myServiceCDep.flags).toEqual({
1010-
optional: false,
1005+
optional: true,
10111006
skipSelf: true,
10121007
self: false,
10131008
host: false,
10141009
});
10151010
expect(myInjectionTokenValueDep.flags).toEqual({
1016-
optional: false,
1011+
optional: true,
10171012
skipSelf: false,
10181013
self: false,
10191014
host: false,
@@ -1045,18 +1040,18 @@ describe('getDependenciesFromInjectable', () => {
10451040

10461041

10471042
expect(dependenciesOfMyStandaloneComponentB[0].value).toBe(myStandalonecomponentB.myService);
1048-
expect(myServiceBDep.value).toBe('hello world');
1049-
expect(myServiceCDep.value).toBe(123);
1050-
expect(myInjectionTokenValueDep.value).toBe(myServiceCInstance);
1043+
expect(myServiceBDep.value).toBe(null);
1044+
expect(myServiceCDep.value).toBe(null);
1045+
expect(myInjectionTokenValueDep.value).toBe(null);
10511046
expect(injectorDep.value).toBe(myStandalonecomponentB.injector);
10521047
expect(myServiceDDep.value).toBe('123');
10531048
expect(myServiceGDep.value).toBe(myStandalonecomponentB.myServiceG);
10541049
expect(parentComponentDep.value).toBe(myStandalonecomponentB.parentComponent);
10551050

1056-
expect(dependenciesOfMyStandaloneComponentB[0].providedIn).toBe(standaloneInjector);
1057-
expect(myServiceBDep.providedIn).toBe(standaloneInjector);
1058-
expect(myServiceCDep.providedIn).toBe(standaloneInjector);
1059-
expect(myInjectionTokenValueDep.providedIn).toBe(standaloneInjector);
1051+
expect(dependenciesOfMyStandaloneComponentB[0].providedIn).toBe(undefined);
1052+
expect(myServiceBDep.providedIn).toBe(undefined);
1053+
expect(myServiceCDep.providedIn).toBe(undefined);
1054+
expect(myInjectionTokenValueDep.providedIn).toBe(undefined);
10601055
expect(injectorDep.providedIn).toBe(myStandalonecomponentB.injector);
10611056
expect(myServiceDDep.providedIn).toBe(standaloneInjector.get(Injector, null, {
10621057
skipSelf: true
@@ -1266,13 +1261,12 @@ describe('getInjectorResolutionPath', () => {
12661261
* NodeInjector[RouterOutlet],
12671262
* NodeInjector[MyStandaloneComponent],
12681263
* R3Injector[LazyComponent],
1269-
* R3Injector[MyStandaloneComponent],
12701264
* R3Injector[Root],
12711265
* R3Injector[Platform],
12721266
* NullInjector
12731267
* ]
12741268
*/
1275-
expect(path.length).toBe(8);
1269+
expect(path.length).toBe(7);
12761270

12771271
expect(path[0]).toBe(lazyComponentNodeInjector);
12781272

@@ -1291,16 +1285,13 @@ describe('getInjectorResolutionPath', () => {
12911285

12921286
expect(path[4]).toBeInstanceOf(R3Injector);
12931287
expect((path[4] as R3Injector).scopes.has('environment')).toBeTrue();
1294-
expect((path[4] as R3Injector).source).toBe('Standalone[MyStandaloneComponent]');
1288+
expect((path[4] as R3Injector).source).toBe('DynamicTestModule');
1289+
expect((path[4] as R3Injector).scopes.has('root')).toBeTrue();
12951290

12961291
expect(path[5]).toBeInstanceOf(R3Injector);
1297-
expect((path[5] as R3Injector).scopes.has('environment')).toBeTrue();
1298-
expect((path[5] as R3Injector).scopes.has('root')).toBeTrue();
1299-
1300-
expect(path[6]).toBeInstanceOf(R3Injector);
1301-
expect((path[6] as R3Injector).scopes.has('platform')).toBeTrue();
1292+
expect((path[5] as R3Injector).scopes.has('platform')).toBeTrue();
13021293

1303-
expect(path[7]).toBeInstanceOf(NullInjector);
1294+
expect(path[6]).toBeInstanceOf(NullInjector);
13041295
}
13051296
}));
13061297
});

‎packages/router/src/directives/router_outlet.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,7 @@ export interface RouterOutletContract {
7171
/**
7272
* Called by the `Router` when the outlet should activate (create a component).
7373
*/
74-
activateWith(
75-
activatedRoute: ActivatedRoute,
76-
environmentInjector: EnvironmentInjector | null,
77-
): void;
74+
activateWith(activatedRoute: ActivatedRoute, environmentInjector: EnvironmentInjector): void;
7875

7976
/**
8077
* A request to destroy the currently activated component.
@@ -216,7 +213,6 @@ export class RouterOutlet implements OnDestroy, OnInit, RouterOutletContract {
216213
private parentContexts = inject(ChildrenOutletContexts);
217214
private location = inject(ViewContainerRef);
218215
private changeDetector = inject(ChangeDetectorRef);
219-
private environmentInjector = inject(EnvironmentInjector);
220216
private inputBinder = inject(INPUT_BINDER, {optional: true});
221217
/** @nodoc */
222218
readonly supportsBindingToComponentInputs = true;
@@ -350,7 +346,7 @@ export class RouterOutlet implements OnDestroy, OnInit, RouterOutletContract {
350346
}
351347
}
352348

353-
activateWith(activatedRoute: ActivatedRoute, environmentInjector?: EnvironmentInjector | null) {
349+
activateWith(activatedRoute: ActivatedRoute, environmentInjector: EnvironmentInjector) {
354350
if (this.isActivated) {
355351
throw new RuntimeError(
356352
RuntimeErrorCode.OUTLET_ALREADY_ACTIVATED,
@@ -368,7 +364,7 @@ export class RouterOutlet implements OnDestroy, OnInit, RouterOutletContract {
368364
this.activated = location.createComponent(component, {
369365
index: location.length,
370366
injector,
371-
environmentInjector: environmentInjector ?? this.environmentInjector,
367+
environmentInjector: environmentInjector,
372368
});
373369
// Calling `markForCheck` to make sure we will run the change detection when the
374370
// `RouterOutlet` is inside a `ChangeDetectionStrategy.OnPush` component.

‎packages/router/src/operators/activate_routes.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ export class ActivateRoutes {
224224
const injector = getClosestRouteInjector(future.snapshot);
225225
context.attachRef = null;
226226
context.route = future;
227-
context.injector = injector;
227+
context.injector = injector ?? context.injector;
228228
if (context.outlet) {
229229
// Activate the outlet when it has already been instantiated
230230
// Otherwise it will get activated from its `ngOnInit` when instantiated

‎packages/router/src/router_outlet_context.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ import {ActivatedRoute} from './router_state';
1919
export class OutletContext {
2020
outlet: RouterOutletContract | null = null;
2121
route: ActivatedRoute | null = null;
22-
injector: EnvironmentInjector | null = null;
23-
children = new ChildrenOutletContexts();
22+
children = new ChildrenOutletContexts(this.injector);
2423
attachRef: ComponentRef<any> | null = null;
24+
constructor(public injector: EnvironmentInjector) {}
2525
}
2626

2727
/**
@@ -34,6 +34,9 @@ export class ChildrenOutletContexts {
3434
// contexts for child outlets, by name.
3535
private contexts = new Map<string, OutletContext>();
3636

37+
/** @nodoc */
38+
constructor(private parentInjector: EnvironmentInjector) {}
39+
3740
/** Called when a `RouterOutlet` directive is instantiated */
3841
onChildOutletCreated(childName: string, outlet: RouterOutletContract): void {
3942
const context = this.getOrCreateContext(childName);
@@ -72,7 +75,7 @@ export class ChildrenOutletContexts {
7275
let context = this.getContext(childName);
7376

7477
if (!context) {
75-
context = new OutletContext();
78+
context = new OutletContext(this.parentInjector);
7679
this.contexts.set(childName, context);
7780
}
7881

‎packages/router/test/directives/router_outlet.spec.ts‎

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,26 @@
77
*/
88

99
import {CommonModule, NgForOf} from '@angular/common';
10-
import {Component, Input, Type} from '@angular/core';
10+
import {
11+
Component,
12+
EnvironmentInjector,
13+
Input,
14+
NgModule,
15+
Type,
16+
createEnvironmentInjector,
17+
importProvidersFrom,
18+
inject,
19+
} from '@angular/core';
1120
import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
1221
import {
1322
provideRouter,
1423
Router,
1524
RouterModule,
1625
RouterOutlet,
1726
withComponentInputBinding,
18-
} from '@angular/router/src';
27+
} from '@angular/router';
1928
import {RouterTestingHarness} from '@angular/router/testing';
29+
import {InjectionToken} from '../../../core/src/di';
2030

2131
describe('router outlet name', () => {
2232
it('should support name binding', fakeAsync(() => {
@@ -381,6 +391,44 @@ describe('component input binding', () => {
381391
});
382392
});
383393

394+
describe('injectors', () => {
395+
it('should always use environment injector from route hierarchy and not inherit from outlet', async () => {
396+
let childTokenValue: any = null;
397+
const TOKEN = new InjectionToken<any>('');
398+
399+
@Component({
400+
template: '',
401+
standalone: true,
402+
})
403+
class Child {
404+
constructor() {
405+
childTokenValue = inject(TOKEN as any, {optional: true});
406+
}
407+
}
408+
409+
@NgModule({
410+
providers: [{provide: TOKEN, useValue: 'some value'}],
411+
})
412+
class ModWithProviders {}
413+
414+
@Component({
415+
template: '<router-outlet/>',
416+
imports: [RouterOutlet, ModWithProviders],
417+
standalone: true,
418+
})
419+
class App {}
420+
421+
TestBed.configureTestingModule({
422+
providers: [provideRouter([{path: 'a', component: Child}])],
423+
});
424+
const fixture = TestBed.createComponent(App);
425+
fixture.detectChanges();
426+
await TestBed.inject(Router).navigateByUrl('/a');
427+
fixture.detectChanges();
428+
expect(childTokenValue).toEqual(null);
429+
});
430+
});
431+
384432
function advance(fixture: ComponentFixture<unknown>, millis?: number): void {
385433
tick(millis);
386434
fixture.detectChanges();

‎packages/router/test/router.spec.ts‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,11 @@ describe('Router', () => {
186186
// Since we only test the guards, we don't need to provide a full navigation
187187
// transition object with all properties set.
188188
const testTransition = {
189-
guards: getAllRouteGuards(futureState, empty, new ChildrenOutletContexts()),
189+
guards: getAllRouteGuards(
190+
futureState,
191+
empty,
192+
new ChildrenOutletContexts(TestBed.inject(EnvironmentInjector)),
193+
),
190194
} as NavigationTransition;
191195

192196
of(testTransition)
@@ -242,7 +246,11 @@ describe('Router', () => {
242246
// Since we only test the guards, we don't need to provide a full navigation
243247
// transition object with all properties set.
244248
const testTransition = {
245-
guards: getAllRouteGuards(futureState, empty, new ChildrenOutletContexts()),
249+
guards: getAllRouteGuards(
250+
futureState,
251+
empty,
252+
new ChildrenOutletContexts(TestBed.inject(EnvironmentInjector)),
253+
),
246254
} as NavigationTransition;
247255

248256
of(testTransition)
@@ -296,7 +304,11 @@ describe('Router', () => {
296304
// Since we only test the guards, we don't need to provide a full navigation
297305
// transition object with all properties set.
298306
const testTransition = {
299-
guards: getAllRouteGuards(futureState, currentState, new ChildrenOutletContexts()),
307+
guards: getAllRouteGuards(
308+
futureState,
309+
currentState,
310+
new ChildrenOutletContexts(TestBed.inject(EnvironmentInjector)),
311+
),
300312
} as NavigationTransition;
301313

302314
of(testTransition)
@@ -368,7 +380,11 @@ describe('Router', () => {
368380
// Since we only test the guards, we don't need to provide a full navigation
369381
// transition object with all properties set.
370382
const testTransition = {
371-
guards: getAllRouteGuards(futureState, currentState, new ChildrenOutletContexts()),
383+
guards: getAllRouteGuards(
384+
futureState,
385+
currentState,
386+
new ChildrenOutletContexts(TestBed.inject(EnvironmentInjector)),
387+
),
372388
} as NavigationTransition;
373389

374390
of(testTransition)
@@ -841,7 +857,7 @@ function checkResolveData(
841857
// Since we only test the guards and their resolve data function, we don't need to provide
842858
// a full navigation transition object with all properties set.
843859
of({
844-
guards: getAllRouteGuards(future, curr, new ChildrenOutletContexts()),
860+
guards: getAllRouteGuards(future, curr, new ChildrenOutletContexts(injector)),
845861
} as NavigationTransition)
846862
.pipe(resolveDataOperator('emptyOnly', injector))
847863
.subscribe(check, (e) => {
@@ -858,7 +874,7 @@ function checkGuards(
858874
// Since we only test the guards, we don't need to provide a full navigation
859875
// transition object with all properties set.
860876
of({
861-
guards: getAllRouteGuards(future, curr, new ChildrenOutletContexts()),
877+
guards: getAllRouteGuards(future, curr, new ChildrenOutletContexts(injector)),
862878
} as NavigationTransition)
863879
.pipe(checkGuardsOperator(injector))
864880
.subscribe({

0 commit comments

Comments
 (0)