From bb75be0bcdd290d01e9bf5872e4d1447ac20a3ea Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 4 Jun 2026 16:40:21 -0700 Subject: [PATCH] refactor(router): Add handling for ActivatedRoute-scoped injector Add handling in navigation for creating and destroying injectors scoped to `ActivatedRoute` life. The code for creating the injectors is certainly more complicated than it _could_ be since there's no actual feature built around this yet. Keeps as much implementation code tree-shakeable as possible: Raw size: +764 bytes Gzipped size: +182 bytes --- .../router/bundle.golden_symbols.json | 2 + .../src/activated_route_injector_feature.ts | 19 ++ packages/router/src/create_router_state.ts | 31 ++- packages/router/src/navigation_transition.ts | 34 ++- .../router/src/operators/activate_routes.ts | 6 + .../setup_activated_route_injectors.ts | 65 +++++ packages/router/src/private_export.ts | 1 + packages/router/src/provide_router.ts | 16 ++ packages/router/src/route_reuse_strategy.ts | 6 + packages/router/src/router_state.ts | 8 + .../test/activated_route_injector.spec.ts | 228 ++++++++++++++++++ .../router/test/create_router_state.spec.ts | 63 ++++- 12 files changed, 460 insertions(+), 19 deletions(-) create mode 100644 packages/router/src/activated_route_injector_feature.ts create mode 100644 packages/router/src/operators/setup_activated_route_injectors.ts create mode 100644 packages/router/test/activated_route_injector.spec.ts diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index ecb572e7b2c2..0b33a7911901 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -1,6 +1,7 @@ { "chunks": { "main": [ + "ACTIVATED_ROUTE_INJECTOR_FEATURE", "AFTER_RENDER_SEQUENCES_TO_ADD", "ANIMATIONS", "ANIMATION_QUEUE", @@ -572,6 +573,7 @@ "diPublicInInjector", "directiveHostEndFirstCreatePass", "directiveHostFirstCreatePass", + "discardNewActivatedRoutes", "documentSupported", "domOnlyFirstCreatePass", "elementAttributeInternal", diff --git a/packages/router/src/activated_route_injector_feature.ts b/packages/router/src/activated_route_injector_feature.ts new file mode 100644 index 000000000000..016d54cfa231 --- /dev/null +++ b/packages/router/src/activated_route_injector_feature.ts @@ -0,0 +1,19 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import {InjectionToken} from '@angular/core'; +import {OperatorFunction} from 'rxjs'; +import type {NavigationTransition} from './navigation_transition'; + +export interface ActivatedRouteInjectorFeature { + operator(): OperatorFunction; +} + +export const ACTIVATED_ROUTE_INJECTOR_FEATURE = new InjectionToken( + typeof ngDevMode === 'undefined' || ngDevMode ? 'ActivatedRoute injector feature' : '', +); diff --git a/packages/router/src/create_router_state.ts b/packages/router/src/create_router_state.ts index aa5960d944f9..c7f1210c6821 100644 --- a/packages/router/src/create_router_state.ts +++ b/packages/router/src/create_router_state.ts @@ -21,21 +21,28 @@ export function createRouterState( routeReuseStrategy: RouteReuseStrategy, curr: RouterStateSnapshot, prevState: RouterState, -): RouterState { - const root = createNode(routeReuseStrategy, curr._root, prevState ? prevState._root : undefined); - return new RouterState(root, curr); +): {newlyCreatedRoutes: Set; state: RouterState} { + const newlyCreatedRoutes = new Set(); + const root = createNode( + routeReuseStrategy, + curr._root, + prevState ? prevState._root : undefined, + newlyCreatedRoutes, + ); + return {newlyCreatedRoutes, state: new RouterState(root, curr)}; } function createNode( routeReuseStrategy: RouteReuseStrategy, curr: TreeNode, - prevState?: TreeNode, + prevState: TreeNode | undefined, + newlyCreatedRoutes: Set, ): TreeNode { // reuse an activated route that is currently displayed on the screen if (prevState && routeReuseStrategy.shouldReuseRoute(curr.value, prevState.value.snapshot)) { const value = prevState.value; value._futureSnapshot = curr.value; - const children = createOrReuseChildren(routeReuseStrategy, curr, prevState); + const children = createOrReuseChildren(routeReuseStrategy, curr, prevState, newlyCreatedRoutes); return new TreeNode(value, children); } else { if (routeReuseStrategy.shouldAttach(curr.value)) { @@ -44,13 +51,18 @@ function createNode( if (detachedRouteHandle !== null) { const tree = (detachedRouteHandle as DetachedRouteHandleInternal).route; tree.value._futureSnapshot = curr.value; - tree.children = curr.children.map((c) => createNode(routeReuseStrategy, c)); + tree.children = curr.children.map((c) => + createNode(routeReuseStrategy, c, undefined, newlyCreatedRoutes), + ); return tree; } } const value = createActivatedRoute(curr.value); - const children = curr.children.map((c) => createNode(routeReuseStrategy, c)); + newlyCreatedRoutes.add(value); + const children = curr.children.map((c) => + createNode(routeReuseStrategy, c, undefined, newlyCreatedRoutes), + ); return new TreeNode(value, children); } } @@ -59,14 +71,15 @@ function createOrReuseChildren( routeReuseStrategy: RouteReuseStrategy, curr: TreeNode, prevState: TreeNode, + newlyCreatedRoutes: Set, ) { return curr.children.map((child) => { for (const p of prevState.children) { if (routeReuseStrategy.shouldReuseRoute(child.value, p.value.snapshot)) { - return createNode(routeReuseStrategy, child, p); + return createNode(routeReuseStrategy, child, p, newlyCreatedRoutes); } } - return createNode(routeReuseStrategy, child); + return createNode(routeReuseStrategy, child, undefined, newlyCreatedRoutes); }); } diff --git a/packages/router/src/navigation_transition.ts b/packages/router/src/navigation_transition.ts index 5de1fd419ec5..721e93e22986 100644 --- a/packages/router/src/navigation_transition.ts +++ b/packages/router/src/navigation_transition.ts @@ -83,6 +83,7 @@ import {UrlSerializer, UrlTree} from './url_tree'; import {abortSignalToObservable} from './utils/abort_signal_to_observable'; import {Checks, getAllRouteGuards} from './utils/preactivation'; import {CREATE_VIEW_TRANSITION} from './utils/view_transition'; +import {ACTIVATED_ROUTE_INJECTOR_FEATURE} from './activated_route_injector_feature'; /** * @description @@ -327,6 +328,7 @@ export interface NavigationTransition { targetRouterState: RouterState | null; guards: Checks; guardsResult: GuardResult | null; + newlyCreatedRoutes?: Set; routesRecognizeHandler: {deferredHandle?: Promise}; beforeActivateHandler: {deferredHandle?: Promise}; @@ -367,6 +369,9 @@ export class NavigationTransitions { private readonly urlHandlingStrategy = inject(UrlHandlingStrategy); private readonly createViewTransition = inject(CREATE_VIEW_TRANSITION, {optional: true}); private readonly navigationErrorHandler = inject(NAVIGATION_ERROR_HANDLER, {optional: true}); + private readonly activatedRouteInjectorFeature = inject(ACTIVATED_ROUTE_INJECTOR_FEATURE, { + optional: true, + }); navigationId = 0; get hasRequestedNavigation() { @@ -740,19 +745,28 @@ export class NavigationTransitions { }), switchMap((t: NavigationTransition) => { - const targetRouterState = createRouterState( + const {newlyCreatedRoutes, state} = createRouterState( router.routeReuseStrategy, t.targetSnapshot!, t.currentRouterState, ); - this.currentTransition = overallTransitionState = t = {...t, targetRouterState}; + this.currentTransition = + overallTransitionState = + t = + { + ...t, + targetRouterState: state, + newlyCreatedRoutes, + }; this.currentNavigation.update((nav) => { - nav!.targetRouterState = targetRouterState; + nav!.targetRouterState = state; return nav; }); return of(t); }), + this.activatedRouteInjectorFeature?.operator() ?? ((t) => t), + switchTap(() => this.afterPreactivation()), // TODO(atscott): Move this into the last block below. @@ -792,6 +806,9 @@ export class NavigationTransitions { this.inputBindingEnabled, ).activate(this.rootContexts); + // Prevent any cleanup of newly created routes once activated. + t.newlyCreatedRoutes?.clear(); + if (!shouldContinueNavigation()) { return; } @@ -876,6 +893,7 @@ export class NavigationTransitions { }), catchError((e) => { completedOrAborted = true; + discardNewActivatedRoutes(overallTransitionState); // If the application is already destroyed, the catch block should not // execute anything in practice because other resources have already // been released and destroyed. @@ -974,6 +992,7 @@ export class NavigationTransitions { reason: string, code: NavigationCancellationCode, ) { + discardNewActivatedRoutes(t); const navCancel = new NavigationCancel( t.id, this.urlSerializer.serialize(t.extractedUrl), @@ -1026,3 +1045,12 @@ export class NavigationTransitions { export function isBrowserTriggeredNavigation(source: NavigationTrigger) { return source !== IMPERATIVE_NAVIGATION; } + +function discardNewActivatedRoutes(t: NavigationTransition): void { + if (!t.newlyCreatedRoutes) { + return; + } + for (const r of t.newlyCreatedRoutes) { + r._localInjector?.destroy(); + } +} diff --git a/packages/router/src/operators/activate_routes.ts b/packages/router/src/operators/activate_routes.ts index 4f4fa548d792..7f0569d3bfc7 100644 --- a/packages/router/src/operators/activate_routes.ts +++ b/packages/router/src/operators/activate_routes.ts @@ -139,6 +139,12 @@ export class ActivateRoutes { context.attachRef = null; context.route = null; } + // Destroy `_localInjector` here when the route is + // unmounted by the Router. This method (`deactivateRouteAndOutlet`) is + // skipped when a route is being detached for `RouteReuseStrategy`, preserving + // its injector. Those preserved injectors are eventually managed and destroyed + // manually via `destroyDetachedRouteHandle()` or if the route is deactivated later rather than detached. + route.value._localInjector?.destroy(); } private activateChildRoutes( diff --git a/packages/router/src/operators/setup_activated_route_injectors.ts b/packages/router/src/operators/setup_activated_route_injectors.ts new file mode 100644 index 000000000000..6583ef309daf --- /dev/null +++ b/packages/router/src/operators/setup_activated_route_injectors.ts @@ -0,0 +1,65 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import {OperatorFunction, pipe, tap} from 'rxjs'; +import {ActivatedRoute, ActivatedRouteSnapshot} from '../router_state'; +import {TreeNode} from '../utils/tree'; +import {NavigationTransition} from '../navigation_transition'; +import {createEnvironmentInjector} from '@angular/core'; + +export function setupActivatedRouteInjectors(): OperatorFunction< + NavigationTransition, + NavigationTransition +> { + return pipe( + tap(({newlyCreatedRoutes, targetRouterState}) => { + if (!newlyCreatedRoutes || !targetRouterState) { + return; + } + + // Obviously the easier way would be to just iterate newlyCreatedRoutes + // and create injectors for them. However, the feature will eventually + // want to do things for routes that are being reused. + const traverse = (stateNode: TreeNode) => { + const route = stateNode.value; + if (route) { + processRoute(route, newlyCreatedRoutes); + } + + for (const childState of stateNode.children) { + traverse(childState); + } + }; + + traverse(targetRouterState._root); + }), + ); +} + +function processRoute(route: ActivatedRoute, newlyCreatedRoutes: Set) { + // Only create injectors for routes with the feature enabled + const useActivatedRouteInjector = (route?.routeConfig as any)?.ɵUseActivatedRouteInjector; + if (!useActivatedRouteInjector) { + return; + } + + if (newlyCreatedRoutes.has(route)) { + setupNewActivatedRouteInjector(route._futureSnapshot, route); + } else { + // TODO: Do something with injectors that already exist + } +} + +function setupNewActivatedRouteInjector(snapshot: ActivatedRouteSnapshot, route: ActivatedRoute) { + if (ngDevMode && !!route._localInjector) { + throw new Error( + 'invalid state: _localInjector should not exist on newly created ActivatedRoute yet', + ); + } + route._localInjector = createEnvironmentInjector([], snapshot._environmentInjector); +} diff --git a/packages/router/src/private_export.ts b/packages/router/src/private_export.ts index b508c826258a..1094335fafee 100644 --- a/packages/router/src/private_export.ts +++ b/packages/router/src/private_export.ts @@ -11,3 +11,4 @@ export {RestoredState as ɵRestoredState} from './navigation_transition'; export {loadChildren as ɵloadChildren} from './router_config_loader'; export {ROUTER_PROVIDERS as ɵROUTER_PROVIDERS} from './router_module'; export {afterNextNavigation as ɵafterNextNavigation} from './utils/navigations'; +export {withActivatedRouteInjectors as ɵwithActivatedRouteInjectors} from './provide_router'; diff --git a/packages/router/src/provide_router.ts b/packages/router/src/provide_router.ts index d0825fabdc20..ab73bfe0f612 100644 --- a/packages/router/src/provide_router.ts +++ b/packages/router/src/provide_router.ts @@ -63,6 +63,8 @@ import { VIEW_TRANSITION_OPTIONS, ViewTransitionsFeatureOptions, } from './utils/view_transition'; +import {ACTIVATED_ROUTE_INJECTOR_FEATURE} from './activated_route_injector_feature'; +import {setupActivatedRouteInjectors} from './operators/setup_activated_route_injectors'; /** * Sets up providers necessary to enable `Router` functionality for the application. @@ -886,6 +888,20 @@ export function withViewTransitions( return routerFeature(RouterFeatureKind.ViewTransitionsFeature, providers); } +export type ActivatedRouteInjectorFeature = + RouterFeature; +export function withActivatedRouteInjectors(): ActivatedRouteInjectorFeature { + const providers = [ + { + provide: ACTIVATED_ROUTE_INJECTOR_FEATURE, + useValue: { + operator: setupActivatedRouteInjectors, + }, + }, + ]; + return routerFeature(RouterFeatureKind.ViewTransitionsFeature, providers); +} + /** * A type alias that represents all Router features available for use with `provideRouter`. * Features can be enabled by adding special functions to the `provideRouter` call. diff --git a/packages/router/src/route_reuse_strategy.ts b/packages/router/src/route_reuse_strategy.ts index 7cada278fef2..7e74bf32bdc5 100644 --- a/packages/router/src/route_reuse_strategy.ts +++ b/packages/router/src/route_reuse_strategy.ts @@ -49,6 +49,12 @@ export function destroyDetachedRouteHandle(handle: DetachedRouteHandle): void { const internalHandle = handle as DetachedRouteHandleInternal; if (internalHandle && internalHandle.componentRef) { internalHandle.componentRef.destroy(); + // It is critical to destroy the `_localInjector` here. When a route is detached + // by the `RouteReuseStrategy`, the `_localInjector` is retained because the + // ActivatedRoute object is stored and can be attached later. + // When the developer drops the handle (e.g., deciding not to reuse it), + // they must manually invoke `destroyDetachedRouteHandle` to prevent a memory leak. + internalHandle.route.value._localInjector?.destroy(); } } diff --git a/packages/router/src/router_state.ts b/packages/router/src/router_state.ts index ec3678bf9ebd..2923541ef63d 100644 --- a/packages/router/src/router_state.ts +++ b/packages/router/src/router_state.ts @@ -154,6 +154,14 @@ export class ActivatedRoute { /** An observable of the static and resolved data of this route. */ public data: Observable; + /** + * Injector scoped to the lifetime of this ActivatedRoute object. + * Created only when features tied to ActivatedRoute lifetime are used. + * + * @internal + */ + _localInjector?: EnvironmentInjector; + /** @internal */ constructor( /** @internal */ diff --git a/packages/router/test/activated_route_injector.spec.ts b/packages/router/test/activated_route_injector.spec.ts new file mode 100644 index 000000000000..b02e15c5ec5b --- /dev/null +++ b/packages/router/test/activated_route_injector.spec.ts @@ -0,0 +1,228 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import {Component, DestroyRef, Injectable} from '@angular/core'; +import {TestBed} from '@angular/core/testing'; +import { + ActivatedRoute, + ActivatedRouteSnapshot, + BaseRouteReuseStrategy, + DetachedRouteHandle, + Route, + RouteReuseStrategy, + Router, + destroyDetachedRouteHandle, + provideRouter, + ɵwithActivatedRouteInjectors, +} from '@angular/router'; +import {RouterTestingHarness} from '@angular/router/testing'; + +describe('ActivatedRoute local injector', () => { + @Component({ + template: 'home', + }) + class HomeComponent {} + + @Component({ + template: 'away', + }) + class AwayComponent {} + + @Injectable({providedIn: 'root'}) + class CustomReuseStrategy extends BaseRouteReuseStrategy { + storedHandles = new Map(); + shouldDetachVal = false; + shouldAttachVal = false; + + override shouldDetach(route: ActivatedRouteSnapshot): boolean { + return this.shouldDetachVal; + } + + override store(route: ActivatedRouteSnapshot, handle: DetachedRouteHandle | null): void { + if (route.routeConfig && handle) { + this.storedHandles.set(route.routeConfig, handle); + } + } + + override shouldAttach(route: ActivatedRouteSnapshot): boolean { + return ( + this.shouldAttachVal && !!route.routeConfig && this.storedHandles.has(route.routeConfig) + ); + } + + override retrieve(route: ActivatedRouteSnapshot): DetachedRouteHandle | null { + return (route.routeConfig && this.storedHandles.get(route.routeConfig)) || null; + } + } + + let router: Router; + let strategy: CustomReuseStrategy; + + async function setUpRouter(routes: Route[]): Promise { + TestBed.configureTestingModule({ + providers: [ + provideRouter(routes, ɵwithActivatedRouteInjectors()), + {provide: RouteReuseStrategy, useClass: CustomReuseStrategy}, + ], + }); + + router = TestBed.inject(Router); + strategy = TestBed.inject(RouteReuseStrategy) as CustomReuseStrategy; + return await RouterTestingHarness.create(); + } + + it('should create and destroy local injector for routes with ɵUseActivatedRouteInjector', async () => { + const harness = await setUpRouter([ + { + path: 'home', + component: HomeComponent, + 'ɵUseActivatedRouteInjector': true, + } as any, + { + path: 'away', + component: AwayComponent, + }, + ]); + + await harness.navigateByUrl('/home'); + + const homeRoute = router.routerState.root.firstChild!; + expect(homeRoute).not.toBeNull(); + const localInjector = (homeRoute as any)._localInjector; + expect(localInjector).toBeDefined(); + + let destroyed = false; + localInjector!.get(DestroyRef).onDestroy(() => { + destroyed = true; + }); + + // Navigate away to trigger deactivation/cleanup + await harness.navigateByUrl('/away'); + + expect(destroyed).toBe(true); + }); + + it('should NOT create local injector for routes without ɵUseActivatedRouteInjector', async () => { + const harness = await setUpRouter([ + { + path: 'home', + component: HomeComponent, + }, + ]); + + await harness.navigateByUrl('/home'); + + const homeRoute = router.routerState.root.firstChild!; + expect(homeRoute).not.toBeNull(); + expect((homeRoute as any)._localInjector).toBeUndefined(); + }); + + it('should keep local injector alive when route is detached and destroy it when handle is destroyed', async () => { + const harness = await setUpRouter([ + { + path: 'home', + component: HomeComponent, + 'ɵUseActivatedRouteInjector': true, + } as any, + { + path: 'away', + component: AwayComponent, + }, + ]); + + strategy.shouldDetachVal = true; + + await harness.navigateByUrl('/home'); + + const homeRoute = router.routerState.root.firstChild!; + const localInjector = (homeRoute as any)._localInjector; + expect(localInjector).toBeDefined(); + + let destroyed = false; + localInjector!.get(DestroyRef).onDestroy(() => { + destroyed = true; + }); + + // Navigate away to detach 'home' + await harness.navigateByUrl('/away'); + + // Verify it is detached and stored + const handle = strategy.storedHandles.get(router.config[0]); + expect(handle).toBeDefined(); + // The injector must NOT be destroyed yet + expect(destroyed).toBe(false); + + // Manually destroy the detached handle + destroyDetachedRouteHandle(handle!); + + expect(destroyed).toBe(true); + }); + + it('should destroy local injectors if navigation fails during activation', async () => { + let throwingRoute: ActivatedRoute | null = null; + let localInjectorInConstructor: any = null; + let throwingRouteDestroyed = false; + + @Component({ + template: '', + }) + class ThrowingComponent { + constructor(route: ActivatedRoute) { + throwingRoute = route; + localInjectorInConstructor = (route as any)._localInjector; + localInjectorInConstructor.get(DestroyRef).onDestroy(() => { + throwingRouteDestroyed = true; + }); + throw new Error('Component instantiation error'); + } + } + + const harness = await setUpRouter([ + { + path: 'home', + component: HomeComponent, + 'ɵUseActivatedRouteInjector': true, + } as any, + { + path: 'throwing', + component: ThrowingComponent, + 'ɵUseActivatedRouteInjector': true, + } as any, + ]); + + await harness.navigateByUrl('/home'); + + const homeRoute = router.routerState.root.firstChild!; + const homeLocalInjector = (homeRoute as any)._localInjector; + expect(homeLocalInjector).toBeDefined(); + + let homeDestroyed = false; + homeLocalInjector!.get(DestroyRef).onDestroy(() => { + homeDestroyed = true; + }); + + // Attempt to navigate to /throwing. This will throw an error during activation. + let threw = false; + try { + await harness.navigateByUrl('/throwing'); + } catch (e) { + threw = true; + } + + expect(threw).toBe(true); + + // Since the navigation failed, we should clean up: + // 1. Home was deactivated, so its injector should be destroyed. + expect(homeDestroyed).toBe(true); + + // 2. The throwing route's injector was created during the transition but failed, + // so it should have been cleaned up (destroyed). + expect(localInjectorInConstructor).toBeDefined(); + expect(throwingRouteDestroyed).toBe(true); + }); +}); diff --git a/packages/router/test/create_router_state.spec.ts b/packages/router/test/create_router_state.spec.ts index 13161ba171fc..585dc35fdb81 100644 --- a/packages/router/test/create_router_state.spec.ts +++ b/packages/router/test/create_router_state.spec.ts @@ -36,7 +36,7 @@ describe('create router state', () => { const emptyState = () => createEmptyState(RootComponent, TestBed.inject(EnvironmentInjector)); it('should create new state', async () => { - const state = createRouterState( + const {state, newlyCreatedRoutes} = createRouterState( reuseStrategy, await createState( [ @@ -55,6 +55,12 @@ describe('create router state', () => { checkActivatedRoute(c[0], ComponentA); checkActivatedRoute(c[1], ComponentB, 'left'); checkActivatedRoute(c[2], ComponentC, 'right'); + + expect(newlyCreatedRoutes.has(state.root)).toBe(false); + expect(newlyCreatedRoutes.has(c[0])).toBe(true); + expect(newlyCreatedRoutes.has(c[1])).toBe(true); + expect(newlyCreatedRoutes.has(c[2])).toBe(true); + expect(newlyCreatedRoutes.size).toBe(3); }); it('should reuse existing nodes when it can', async () => { @@ -64,13 +70,13 @@ describe('create router state', () => { {path: 'c', component: ComponentC, outlet: 'left'}, ]; - const prevState = createRouterState( + const {state: prevState, newlyCreatedRoutes: prevCreated} = createRouterState( reuseStrategy, await createState(config, 'a(left:b)'), emptyState(), ); advanceState(prevState); - const state = createRouterState( + const {state, newlyCreatedRoutes} = createRouterState( reuseStrategy, await createState(config, 'a(left:c)'), prevState, @@ -83,6 +89,16 @@ describe('create router state', () => { expect(prevC[0]).toBe(currC[0]); expect(prevC[1]).not.toBe(currC[1]); checkActivatedRoute(currC[1], ComponentC, 'left'); + + expect(prevCreated.has(prevState.root)).toBe(false); + expect(prevCreated.has(prevC[0])).toBe(true); + expect(prevCreated.has(prevC[1])).toBe(true); + expect(prevCreated.size).toBe(2); + + expect(newlyCreatedRoutes.has(state.root)).toBe(false); + expect(newlyCreatedRoutes.has(currC[0])).toBe(false); + expect(newlyCreatedRoutes.has(currC[1])).toBe(true); + expect(newlyCreatedRoutes.size).toBe(1); }); it('should handle componentless routes', async () => { @@ -96,13 +112,13 @@ describe('create router state', () => { }, ]; - const prevState = createRouterState( + const {state: prevState} = createRouterState( reuseStrategy, await createState(config, 'a/1;p=11/(b//right:c)'), emptyState(), ); advanceState(prevState); - const state = createRouterState( + const {state} = createRouterState( reuseStrategy, await createState(config, 'a/2;p=22/(b//right:c)'), prevState, @@ -130,7 +146,7 @@ describe('create router state', () => { ]; spyOn(reuseStrategy, 'retrieve'); - const prevState = createRouterState( + const {state: prevState} = createRouterState( reuseStrategy, await createState(config, 'a(left:b)'), emptyState(), @@ -146,7 +162,7 @@ describe('create router state', () => { {path: 'product/:id', component: ComponentB}, ]; spyOn(reuseStrategy, 'shouldReuseRoute').and.callThrough(); - const previousState = createRouterState( + const {state: previousState} = createRouterState( reuseStrategy, await createState(config, ''), emptyState(), @@ -170,6 +186,39 @@ describe('create router state', () => { expect(current2._routerState.url).toEqual(tree('').toString()); expect(future2._routerState.url).toEqual(tree('product/30').toString()); }); + + it('should not include attached routes in newlyCreatedRoutes', async () => { + const config = [{path: 'a', component: ComponentA}]; + + // First transition creates the route + const {state: state1, newlyCreatedRoutes: created1} = createRouterState( + reuseStrategy, + await createState(config, 'a'), + emptyState(), + ); + const routeA = (state1 as any).firstChild(state1.root); + expect(created1.has(routeA)).toBe(true); + + // Simulate detaching routeA + const detachedHandle = { + route: new TreeNode(routeA, []), + } as any; + + spyOn(reuseStrategy, 'shouldAttach').and.returnValue(true); + spyOn(reuseStrategy, 'retrieve').and.returnValue(detachedHandle); + + // Second transition attaches the route + const {state: state2, newlyCreatedRoutes: created2} = createRouterState( + reuseStrategy, + await createState(config, 'a'), + emptyState(), + ); + + const child2 = (state2 as any).firstChild(state2.root); + expect(child2).toBe(routeA); // attached route + expect(created2.has(routeA)).toBe(false); // not in newlyCreatedRoutes of the second transition + expect(created2.size).toBe(0); + }); }); function advanceState(state: RouterState): void {