refactor(router): clean up route resource implementation and test helpers

Clean up router resource handling:
- Simplify reload method handling on router resource wrappers
- Clean up resources access in outlet input binding effects and component input binder
- Extract waitForResource helper for tracking blocking resource readiness
- Simplify setupNewRouterResources and updateExistingResources
- Remove redundant fixture.whenStable() calls in router resource tests

Author: atscott

packages/router/src/directives/router_outlet.ts

@@ -516,7 +516,7 @@ export class RoutedComponentInputBinder {
             ...queryParams,
             ...params,
             ...data,
-            ...(activatedRoute.resources || {}),
+            ...activatedRoute.resources,
           };
           // Get the first result from the data subscription synchronously so it's available to
           // the component as soon as possible (and doesn't require a second change detection).

packages/router/src/operators/setup_and_run_resources.ts

@@ -11,9 +11,10 @@ import {
   Resource,
   effect,
   DestroyRef,
+  EnvironmentInjector,
 } from '@angular/core';
 import {OperatorFunction, pipe} from 'rxjs';
-import {ResourceContext, ResourceResult} from '../models';
+import {ResourceContext, ResourceResult, Route} from '../models';
 import {NavigationTransition} from '../navigation_transition';
 import {ActivatedRoute, ActivatedRouteSnapshot, initializeActivatedRoute} from '../router_state';
 import {TreeNode} from '../utils/tree';
@@ -77,24 +78,20 @@ function processRoute(
   if (newlyCreatedRoutes.has(route)) {
     // This route is new. We need to run its resources function once.
     resourceSetupPromises.push(
-      setupNewRouterResources(route._futureSnapshot, route, abortSignal, blockingResourcePromises),
+      setupNewRouterResources(route, resources, abortSignal, blockingResourcePromises),
     );
   } else {
     updateExistingResources(route, blockingResourcePromises, abortSignal);
   }
 }
 
 async function setupNewRouterResources(
-  snapshot: ActivatedRouteSnapshot,
   route: ActivatedRoute,
+  resourcesFn: NonNullable<Route['resources']>,
   abortSignal: AbortSignal,
   blockingResourcePromises: Promise<void>[],
 ) {
-  const resourcesFn = snapshot?.routeConfig?.resources;
-  const parentInjector = snapshot?._environmentInjector;
-  if (!resourcesFn || !parentInjector) {
-    return;
-  }
+  const parentInjector = route._futureSnapshot._environmentInjector;
 
   let childInjector = route._localInjector;
   if (!childInjector) {
@@ -109,37 +106,27 @@ async function setupNewRouterResources(
     data: route.dataSignal,
   };
 
-  const resourceResultRaw = runInInjectionContext(childInjector, () => resourcesFn(context));
-  let resourceResult: ResourceResult;
-  if (resourceResultRaw instanceof Promise) {
-    resourceResult = await resourceResultRaw;
-    // Bail out if the router cancelled the navigation (and destroyed our injector!)
-    // while we were waiting.
-    if (abortSignal.aborted) return;
-  } else {
-    resourceResult = resourceResultRaw as ResourceResult;
-  }
-
-  if (!resourceResult) return;
+  const resourceResult = await runInInjectionContext(childInjector, () => resourcesFn(context));
+  if (abortSignal.aborted || !resourceResult) return;
 
   const wrappedResult: ResourceResult = {};
-  for (const [key, res] of Object.entries(resourceResult)) {
-    if (typeof ngDevMode === 'undefined' || ngDevMode) {
+  runInInjectionContext(childInjector, () => {
+    for (const [key, res] of Object.entries(resourceResult)) {
       if (
-        !res ||
-        typeof res !== 'object' ||
-        typeof (res as Partial<Resource<unknown>>).snapshot !== 'function'
+        (typeof ngDevMode === 'undefined' || ngDevMode) &&
+        (!res ||
+          typeof res !== 'object' ||
+          typeof (res as Partial<Resource<unknown>>).snapshot !== 'function')
       ) {
         throw new Error(
           `Invalid resource returned for key "${key}". Expected a Resource, but got ${res === null ? 'null' : typeof res}.`,
         );
       }
+      wrappedResult[key] = routerResource(res);
     }
+  });
 
-    wrappedResult[key] = runInInjectionContext(childInjector, () => routerResource(res));
-  }
-
-  route.resources = route._futureSnapshot.resources = snapshot.resources = wrappedResult;
+  route.resources = route._futureSnapshot.resources = wrappedResult;
   setupBlocking(route, wrappedResult, blockingResourcePromises, abortSignal);
 }
 
@@ -155,15 +142,15 @@ function updateExistingResources(
     return;
   }
 
-  Object.values(currentResources).forEach((r) => {
+  for (const r of Object.values(currentResources)) {
     const underlyingRes = (r as InternalRouterResource)[SOURCE_RESOURCE_SYMBOL];
     if (underlyingRes.status() === 'error') {
       // If a resource previously failed and the route is reused identically,
       // the parameter signals won't change, meaning the internal effect won't automatically refetch.
       // We must manually trigger a reload to ensure the new navigation attempts a retry.
-      (underlyingRes as unknown as {reload?: () => boolean}).reload?.();
+      (underlyingRes as Partial<InternalRouterResource>).reload?.();
     }
-  });
+  }
 
   route._futureSnapshot.resources = currentResources;
   setupBlocking(route, currentResources, blockingResourcePromises, abortSignal);
@@ -179,52 +166,53 @@ function setupBlocking(
   const childInjector = route._localInjector;
   if (!childInjector || !resourceResult) return;
 
-  for (const r of Object.values(resourceResult)) {
-    const res = r as InternalRouterResource;
-    if (res[BLOCKING_SYMBOL] === false) {
-      continue;
+  for (const res of Object.values(resourceResult)) {
+    const internalRes = res as InternalRouterResource;
+    if (internalRes[BLOCKING_SYMBOL] !== false) {
+      blockingResourcePromises.push(waitForResource(internalRes, childInjector, abortSignal));
     }
-    const promise = new Promise<void>((resolve, reject) => {
-      const underlyingRes = res[SOURCE_RESOURCE_SYMBOL];
-      let isDestroyed = false;
-      let unregisterOnDestroy: (() => void) | undefined;
-
-      const cleanup = () => {
-        isDestroyed = true;
-        blockingEffect.destroy();
-        unregisterOnDestroy?.();
-        abortSignal.removeEventListener('abort', onAbort);
-      };
+  }
+}
 
-      const onAbort = () => {
-        cleanup();
-        resolve();
-      };
+function waitForResource(
+  resource: InternalRouterResource,
+  injector: EnvironmentInjector,
+  abortSignal: AbortSignal,
+): Promise<void> {
+  return new Promise<void>((resolve, reject) => {
+    const underlyingRes = resource[SOURCE_RESOURCE_SYMBOL];
+    let isDestroyed = false;
+    let unregisterDestroy: (() => void) | undefined;
+
+    const cleanup = () => {
+      isDestroyed = true;
+      blockingEffect.destroy();
+      unregisterDestroy?.();
+      abortSignal.removeEventListener('abort', onDone);
+    };
+
+    const onDone = () => {
+      cleanup();
+      resolve();
+    };
+
+    abortSignal.addEventListener('abort', onDone, {once: true});
+
+    const blockingEffect = effect(
+      () => {
+        if (isDestroyed) return;
+        const status = underlyingRes.status();
+        if (status === 'error') {
+          cleanup();
+          reject(underlyingRes.error());
+        } else if (!underlyingRes.isLoading()) {
+          cleanup();
+          resolve();
+        }
+      },
+      {injector, manualCleanup: true},
+    );
 
-      abortSignal.addEventListener('abort', onAbort, {once: true});
-
-      const blockingEffect = effect(
-        () => {
-          if (isDestroyed) {
-            return;
-          }
-          const status = underlyingRes.status();
-          if (status === 'error') {
-            cleanup();
-            reject(underlyingRes.error());
-          } else if (!underlyingRes.isLoading()) {
-            cleanup();
-            resolve();
-          }
-        },
-        {injector: childInjector, manualCleanup: true},
-      );
-
-      unregisterOnDestroy = childInjector.get(DestroyRef).onDestroy(() => {
-        cleanup();
-        resolve();
-      });
-    });
-    blockingResourcePromises.push(promise);
-  }
+    unregisterDestroy = injector.get(DestroyRef).onDestroy(onDone);
+  });
 }

packages/router/src/router_resource.ts

@@ -80,22 +80,18 @@ export function routerResource<T>(source: Resource<T>): Resource<T> & {reload():
   res[BLOCKING_SYMBOL] =
     (source as unknown as InternalRouterResource<T>)[BLOCKING_SYMBOL] !== false;
 
-  if (typeof (source as any).reload === 'function') {
-    res.reload = function (): boolean {
-      // If the resource is currently frozen (e.g., during an active navigation transition
-      // or while recovering from a cancelled navigation), we reject manual reload requests.
-      // Triggering a reload during an active navigation (where the resource may already be
-      // reactively loading new parameters behind the scenes) would disrupt the router's resource
-      // tracking for the transition. Similarly, during a rollback recovery, the router is
-      // already managing the resource reload to restore the previous state.
-      if (frozenSnapshot() !== null) {
-        return false;
-      }
-      return (source as any).reload();
-    };
-  } else {
-    res.reload = () => false;
-  }
+  res.reload = function (): boolean {
+    // If the resource is currently frozen (e.g., during an active navigation transition
+    // or while recovering from a cancelled navigation), we reject manual reload requests.
+    // Triggering a reload during an active navigation (where the resource may already be
+    // reactively loading new parameters behind the scenes) would disrupt the router's resource
+    // tracking for the transition. Similarly, during a rollback recovery, the router is
+    // already managing the resource reload to restore the previous state.
+    if (frozenSnapshot() !== null) {
+      return false;
+    }
+    return (source as any).reload?.() ?? false;
+  };
 
   return res;
 }
@@ -193,12 +189,13 @@ export function createResourceOutletBindingEffects(
   const createdEffects: EffectRef[] = [];
   const handledKeys: string[] = [];
   const mirror = route.component ? reflectComponentType(route.component) : null;
-  if (!mirror) {
+  const resources = route.resources;
+  if (!mirror || !resources) {
     return {createdEffects, handledKeys};
   }
 
   for (const {templateName} of mirror.inputs) {
-    const resource = route.resources?.[templateName];
+    const resource = resources[templateName];
     if (!resource || !(resource as InternalRouterResource)[BLOCKING_SYMBOL]) {
       continue;
     }

packages/router/test/router_resource_spec.ts

@@ -128,7 +128,6 @@ describe('Router resources integration', () => {
       ]);
 
       await harness.navigateByUrl('/test');
-      await harness.fixture.whenStable();
 
       const route = router.routerState.root.firstChild;
       const resourceRef = route?.resources?.['data'];
@@ -163,7 +162,6 @@ describe('Router resources integration', () => {
       // 2. Now resolve the blocking resource loader
       loaderDeferred.resolve('resolved data');
       await navPromise;
-      await harness.fixture.whenStable();
 
       expect(router.url).toBe('/test');
       const route = router.routerState.root.firstChild;
@@ -313,7 +311,6 @@ describe('Router resources integration', () => {
       ]);
 
       await harness.navigateByUrl('/parent/componentless/child');
-      await harness.fixture.whenStable();
       await timeout(20);
 
       const parentRoute = router.routerState.root.firstChild!;
@@ -534,7 +531,6 @@ describe('Router resources integration', () => {
 
       // Navigate without query params -> params() is undefined -> resource is idle
       await harness.navigateByUrl('/search');
-      await harness.fixture.whenStable();
 
       expect(router.url).toBe('/search');
       const resourceRef = router.routerState.root.firstChild?.resources?.['data'];
@@ -562,8 +558,7 @@ describe('Router resources integration', () => {
       ]);
 
       await harness.navigateByUrl('/test');
-      await harness.fixture.whenStable();
-      const resourceRef = router.routerState.root.firstChild?.resources?.['data'];
+      const resourceRef = router.routerState.root.firstChild?.resources?.['data'] as any;
 
       await timeout(20);
       expect(resourceRef?.value()).toEqual({name: 'user 123'});
@@ -601,7 +596,6 @@ describe('Router resources integration', () => {
       // Fail next navigation
       canActivate = false;
       await harness.navigateByUrl('/test/2');
-      await harness.fixture.whenStable();
 
       // The navigation is cancelled so the resource should retain the old value without loading flicker.
       expect(resourceRef?.value()).toBe('1');
@@ -740,7 +734,6 @@ describe('Router resources integration', () => {
 
       p3.resolve('loaded-3');
       await nav3;
-      await harness.fixture.whenStable();
 
       const resourceRef = router.routerState.root.firstChild?.resources?.['data'];
       expect(resourceRef?.value()).toBe('loaded-3');
@@ -878,7 +871,6 @@ describe('Router resources integration', () => {
       expect(resourceRef?.value()).toBeUndefined();
 
       await nav;
-      await harness.fixture.whenStable();
 
       expect(resourceRef?.isLoading()).toBe(false);
       expect(resourceRef?.value()).toBe('rx loaded 123');
@@ -973,7 +965,6 @@ describe('Router resources integration', () => {
         // Resolve user
         resolveUser({id: 'u1', teamId: 't42'});
         await nav;
-        await harness.fixture.whenStable();
 
         expect(router.url).toBe('/user-team');
         const resources = router.routerState.root.firstChild?.resources;
@@ -1036,7 +1027,6 @@ describe('Router resources integration', () => {
         // Resolve child
         resolveChild({role: 'admin', permissions: ['read', 'write']});
         await nav;
-        await harness.fixture.whenStable();
 
         expect(router.url).toBe('/user/1/details');
         const parentRoute = router.routerState.root.firstChild!;
@@ -1063,7 +1053,7 @@ describe('Router resources integration', () => {
           }
         }
 
-        const {harness, router} = await setupRouter([
+        const {harness} = await setupRouter([
           {
             path: 'user',
             component: UserProfileCmp,
@@ -1076,14 +1066,13 @@ describe('Router resources integration', () => {
         ]);
 
         const instance = await harness.navigateByUrl('/user', UserProfileCmp);
-        await harness.fixture.whenStable();
 
         expect(fetchCount).toBe(1);
         expect(instance.userResource?.value()).toBe('User #1');
 
         const reloaded = instance.refreshUser();
         expect(reloaded).toBe(true);
-        await harness.fixture.whenStable();
+        await timeout();
 
         expect(fetchCount).toBe(2);
         expect(instance.userResource?.value()).toBe('User #2');
@@ -1136,7 +1125,6 @@ describe('Router resources integration', () => {
         ]);
 
         await harness.navigateByUrl('/custom');
-        await harness.fixture.whenStable();
 
         const route = router.routerState.root.firstChild!;
         expect(route.resources?.['custom']?.reload()).toBe(false);