From b7d3ecc873b2cafe45ffa1bbfc8cfeda4b4c9e6b Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Mon, 1 Jul 2024 15:30:12 -0700 Subject: [PATCH] fix(router): routes should not get stale providers (#56798) This fixes a bug with RouterOutlet and its context where it would reuse providers from a previously activated route. fixes #56774 PR Close #56798 --- goldens/public-api/router/index.api.md | 7 ++-- .../router/src/components/empty_outlet.ts | 21 +++++++++- .../router/src/operators/activate_routes.ts | 2 - packages/router/src/router.ts | 3 +- packages/router/src/router_config_loader.ts | 3 +- packages/router/src/router_outlet_context.ts | 15 ++++++-- packages/router/src/utils/config.ts | 21 +--------- .../test/directives/router_outlet.spec.ts | 38 +++++++++++++++++++ 8 files changed, 78 insertions(+), 32 deletions(-) diff --git a/goldens/public-api/router/index.api.md b/goldens/public-api/router/index.api.md index 72b478f157e..44f37b47e2d 100644 --- a/goldens/public-api/router/index.api.md +++ b/goldens/public-api/router/index.api.md @@ -183,7 +183,7 @@ export class ChildActivationStart { // @public export class ChildrenOutletContexts { - constructor(parentInjector: EnvironmentInjector); + constructor(rootInjector: EnvironmentInjector); // (undocumented) getContext(childName: string): OutletContext | null; // (undocumented) @@ -530,13 +530,14 @@ export type OnSameUrlNavigation = 'reload' | 'ignore'; // @public export class OutletContext { - constructor(injector: EnvironmentInjector); + constructor(rootInjector: EnvironmentInjector); // (undocumented) attachRef: ComponentRef | null; // (undocumented) children: ChildrenOutletContexts; // (undocumented) - injector: EnvironmentInjector; + get injector(): EnvironmentInjector; + set injector(_: EnvironmentInjector); // (undocumented) outlet: RouterOutletContract | null; // (undocumented) diff --git a/packages/router/src/components/empty_outlet.ts b/packages/router/src/components/empty_outlet.ts index 714ee5d6401..62185d59fb2 100644 --- a/packages/router/src/components/empty_outlet.ts +++ b/packages/router/src/components/empty_outlet.ts @@ -9,6 +9,9 @@ import {Component} from '@angular/core'; import {RouterOutlet} from '../directives/router_outlet'; +import {PRIMARY_OUTLET} from '../shared'; +import {Route} from '../models'; +export {ɵEmptyOutletComponent as EmptyOutletComponent}; /** * This component is used internally within the router to be a placeholder when an empty @@ -26,4 +29,20 @@ import {RouterOutlet} from '../directives/router_outlet'; }) export class ɵEmptyOutletComponent {} -export {ɵEmptyOutletComponent as EmptyOutletComponent}; +/** + * Makes a copy of the config and adds any default required properties. + */ +export function standardizeConfig(r: Route): Route { + const children = r.children && r.children.map(standardizeConfig); + const c = children ? {...r, children} : {...r}; + if ( + !c.component && + !c.loadComponent && + (children || c.loadChildren) && + c.outlet && + c.outlet !== PRIMARY_OUTLET + ) { + c.component = ɵEmptyOutletComponent; + } + return c; +} diff --git a/packages/router/src/operators/activate_routes.ts b/packages/router/src/operators/activate_routes.ts index f51b86df90c..9d4e4a3a872 100644 --- a/packages/router/src/operators/activate_routes.ts +++ b/packages/router/src/operators/activate_routes.ts @@ -221,10 +221,8 @@ export class ActivateRoutes { advanceActivatedRoute(stored.route.value); this.activateChildRoutes(futureNode, null, context.children); } else { - const injector = getClosestRouteInjector(future.snapshot); context.attachRef = null; context.route = future; - 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 diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts index 665eab460ee..da7c71e1abb 100644 --- a/packages/router/src/router.ts +++ b/packages/router/src/router.ts @@ -54,8 +54,9 @@ import { UrlSerializer, UrlTree, } from './url_tree'; -import {standardizeConfig, validateConfig} from './utils/config'; +import {validateConfig} from './utils/config'; import {afterNextNavigation} from './utils/navigations'; +import {standardizeConfig} from './components/empty_outlet'; function defaultErrorHandler(error: any): never { throw error; diff --git a/packages/router/src/router_config_loader.ts b/packages/router/src/router_config_loader.ts index c011c7fbb70..3b8515691b9 100644 --- a/packages/router/src/router_config_loader.ts +++ b/packages/router/src/router_config_loader.ts @@ -21,7 +21,8 @@ import {finalize, map, mergeMap, refCount, tap} from 'rxjs/operators'; import {DefaultExport, LoadedRouterConfig, Route, Routes} from './models'; import {wrapIntoObservable} from './utils/collection'; -import {assertStandalone, standardizeConfig, validateConfig} from './utils/config'; +import {assertStandalone, validateConfig} from './utils/config'; +import {standardizeConfig} from './components/empty_outlet'; /** * The DI token for a router configuration. diff --git a/packages/router/src/router_outlet_context.ts b/packages/router/src/router_outlet_context.ts index e59c4d82f77..78c305ea93f 100644 --- a/packages/router/src/router_outlet_context.ts +++ b/packages/router/src/router_outlet_context.ts @@ -10,6 +10,7 @@ import {ComponentRef, EnvironmentInjector, Injectable} from '@angular/core'; import {RouterOutletContract} from './directives/router_outlet'; import {ActivatedRoute} from './router_state'; +import {getClosestRouteInjector} from './utils/config'; /** * Store contextual information about a `RouterOutlet` @@ -19,9 +20,15 @@ import {ActivatedRoute} from './router_state'; export class OutletContext { outlet: RouterOutletContract | null = null; route: ActivatedRoute | null = null; - children = new ChildrenOutletContexts(this.injector); + children = new ChildrenOutletContexts(this.rootInjector); attachRef: ComponentRef | null = null; - constructor(public injector: EnvironmentInjector) {} + get injector(): EnvironmentInjector { + return getClosestRouteInjector(this.route?.snapshot) ?? this.rootInjector; + } + // TODO(atscott): Only here to avoid a "breaking" change in a patch/minor. Remove in v19. + set injector(_: EnvironmentInjector) {} + + constructor(private readonly rootInjector: EnvironmentInjector) {} } /** @@ -35,7 +42,7 @@ export class ChildrenOutletContexts { private contexts = new Map(); /** @nodoc */ - constructor(private parentInjector: EnvironmentInjector) {} + constructor(private rootInjector: EnvironmentInjector) {} /** Called when a `RouterOutlet` directive is instantiated */ onChildOutletCreated(childName: string, outlet: RouterOutletContract): void { @@ -75,7 +82,7 @@ export class ChildrenOutletContexts { let context = this.getContext(childName); if (!context) { - context = new OutletContext(this.parentInjector); + context = new OutletContext(this.rootInjector); this.contexts.set(childName, context); } diff --git a/packages/router/src/utils/config.ts b/packages/router/src/utils/config.ts index 80972c02735..81e97b7a7ee 100644 --- a/packages/router/src/utils/config.ts +++ b/packages/router/src/utils/config.ts @@ -15,7 +15,6 @@ import { ɵRuntimeError as RuntimeError, } from '@angular/core'; -import {EmptyOutletComponent} from '../components/empty_outlet'; import {RuntimeErrorCode} from '../errors'; import {Route, Routes} from '../models'; import {ActivatedRouteSnapshot} from '../router_state'; @@ -222,24 +221,6 @@ function getFullPath(parentPath: string, currentRoute: Route): string { } } -/** - * Makes a copy of the config and adds any default required properties. - */ -export function standardizeConfig(r: Route): Route { - const children = r.children && r.children.map(standardizeConfig); - const c = children ? {...r, children} : {...r}; - if ( - !c.component && - !c.loadComponent && - (children || c.loadChildren) && - c.outlet && - c.outlet !== PRIMARY_OUTLET - ) { - c.component = EmptyOutletComponent; - } - return c; -} - /** Returns the `route.outlet` or PRIMARY_OUTLET if none exists. */ export function getOutlet(route: Route): string { return route.outlet || PRIMARY_OUTLET; @@ -268,7 +249,7 @@ export function sortByMatchingOutlets(routes: Routes, outletName: string): Route * also used for getting the correct injector to use for creating components. */ export function getClosestRouteInjector( - snapshot: ActivatedRouteSnapshot, + snapshot: ActivatedRouteSnapshot | undefined, ): EnvironmentInjector | null { if (!snapshot) return null; diff --git a/packages/router/test/directives/router_outlet.spec.ts b/packages/router/test/directives/router_outlet.spec.ts index 8de1cd15825..cd0c59a4f63 100644 --- a/packages/router/test/directives/router_outlet.spec.ts +++ b/packages/router/test/directives/router_outlet.spec.ts @@ -427,6 +427,44 @@ describe('injectors', () => { fixture.detectChanges(); expect(childTokenValue).toEqual(null); }); + + it('should not get sibling providers', async () => { + let childTokenValue: any = null; + const TOKEN = new InjectionToken(''); + @Component({ + template: '', + standalone: true, + }) + class Child { + constructor() { + childTokenValue = inject(TOKEN, {optional: true}); + } + } + + @Component({ + template: '', + imports: [RouterOutlet], + standalone: true, + }) + class App {} + + TestBed.configureTestingModule({ + providers: [ + provideRouter([ + {path: 'a', providers: [{provide: TOKEN, useValue: 'a value'}], component: Child}, + {path: 'b', component: Child}, + ]), + ], + }); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + await TestBed.inject(Router).navigateByUrl('/a'); + fixture.detectChanges(); + expect(childTokenValue).toEqual('a value'); + await TestBed.inject(Router).navigateByUrl('/b'); + fixture.detectChanges(); + expect(childTokenValue).toEqual(null); + }); }); function advance(fixture: ComponentFixture, millis?: number): void {