Sitelet https://github.com/angular/angular/commit/d2e3bac33b022679e596ed388eaefa316a0837e9
Skip to content

Commit d2e3bac

Browse files
atscottleonsenft
atscott
authored and
leonsenft
committed
refactor(router): add support for blocking router resources
Extends router resource integration to support blocking resources during navigation transitions. (cherry picked from commit fa2aca9)
1 parent 34f0e13 commit d2e3bac

4 files changed

Lines changed: 628 additions & 601 deletions

File tree

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

Lines changed: 96 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,31 +5,50 @@
55
* Use of this source code is governed by an MIT-style license that can be
66
* found in the LICENSE file at https://angular.dev/license
77
*/
8-
import {createEnvironmentInjector, runInInjectionContext, Resource} from '@angular/core';
8+
import {
9+
createEnvironmentInjector,
10+
runInInjectionContext,
11+
Resource,
12+
effect,
13+
DestroyRef,
14+
} from '@angular/core';
915
import {OperatorFunction, pipe} from 'rxjs';
1016
import {ResourceContext, ResourceResult} from '../models';
1117
import {NavigationTransition} from '../navigation_transition';
1218
import {ActivatedRoute, ActivatedRouteSnapshot, initializeActivatedRoute} from '../router_state';
1319
import {TreeNode} from '../utils/tree';
14-
import {BLOCKING_SYMBOL, InternalRouterResource, routerResource} from '../router_resource';
20+
import {
21+
BLOCKING_SYMBOL,
22+
hasValueOrResolved,
23+
InternalRouterResource,
24+
routerResource,
25+
SOURCE_RESOURCE_SYMBOL,
26+
} from '../router_resource';
1527
import {switchTap} from './switch_tap';
1628

1729
export function setupAndRunResources(
1830
abortSignal: AbortSignal,
1931
): OperatorFunction<NavigationTransition, NavigationTransition> {
2032
return pipe(
2133
switchTap(({newlyCreatedRoutes, targetRouterState}) => {
22-
if (!newlyCreatedRoutes || !targetRouterState) {
34+
if (!newlyCreatedRoutes || !targetRouterState || abortSignal.aborted) {
2335
return;
2436
}
2537

2638
const resourceSetupPromises: Array<Promise<void>> = [];
39+
const blockingResourcePromises: Array<Promise<void>> = [];
2740

2841
const traverse = (stateNode: TreeNode<ActivatedRoute>) => {
2942
const route = stateNode.value;
3043
if (route) {
3144
initializeActivatedRoute(route);
32-
processRoute(route, newlyCreatedRoutes, resourceSetupPromises, abortSignal);
45+
processRoute(
46+
route,
47+
newlyCreatedRoutes,
48+
resourceSetupPromises,
49+
abortSignal,
50+
blockingResourcePromises,
51+
);
3352
}
3453

3554
for (const childState of stateNode.children) {
@@ -39,8 +58,7 @@ export function setupAndRunResources(
3958

4059
traverse(targetRouterState._root);
4160

42-
return Promise.all(resourceSetupPromises);
43-
// TODO: wait for blocking resources
61+
return Promise.all(resourceSetupPromises).then(() => Promise.all(blockingResourcePromises));
4462
}),
4563
);
4664
}
@@ -50,6 +68,7 @@ function processRoute(
5068
newlyCreatedRoutes: Set<ActivatedRoute>,
5169
resourceSetupPromises: Array<Promise<void>>,
5270
abortSignal: AbortSignal,
71+
blockingResourcePromises: Array<Promise<void>>,
5372
) {
5473
const resources = route.routeConfig?.resources;
5574
if (!resources) {
@@ -58,16 +77,19 @@ function processRoute(
5877

5978
if (newlyCreatedRoutes.has(route)) {
6079
// This route is new. We need to run its resources function once.
61-
resourceSetupPromises.push(setupNewRouterResources(route._futureSnapshot, route, abortSignal));
80+
resourceSetupPromises.push(
81+
setupNewRouterResources(route._futureSnapshot, route, abortSignal, blockingResourcePromises),
82+
);
6283
} else {
63-
updateExistingResources(route);
84+
updateExistingResources(route, blockingResourcePromises, abortSignal);
6485
}
6586
}
6687

6788
async function setupNewRouterResources(
6889
snapshot: ActivatedRouteSnapshot,
6990
route: ActivatedRoute,
7091
abortSignal: AbortSignal,
92+
blockingResourcePromises: Promise<void>[],
7193
) {
7294
const resourcesFn = snapshot?.routeConfig?.resources;
7395
const parentInjector = snapshot?._environmentInjector;
@@ -120,22 +142,42 @@ async function setupNewRouterResources(
120142
}
121143

122144
route.resources = route._futureSnapshot.resources = snapshot.resources = wrappedResult;
123-
prohibitBlockingResources(route, wrappedResult);
145+
setupBlocking(route, wrappedResult, blockingResourcePromises, abortSignal);
124146
}
125147

126-
function updateExistingResources(route: ActivatedRoute) {
148+
function updateExistingResources(
149+
route: ActivatedRoute,
150+
blockingResourcePromises: Promise<void>[],
151+
abortSignal: AbortSignal,
152+
) {
127153
// This route is reused. We must eagerly update the resource context signals
128154
// so that resources can react and fetch new data during the pending navigation.
129155
const currentResources = route.snapshot?.resources;
130156
if (!currentResources) {
131157
return;
132158
}
133159

160+
Object.values(currentResources).forEach((r) => {
161+
const underlyingRes = (r as InternalRouterResource)[SOURCE_RESOURCE_SYMBOL];
162+
if (underlyingRes.status() === 'error') {
163+
// If a resource previously failed and the route is reused identically,
164+
// the parameter signals won't change, meaning the internal effect won't automatically refetch.
165+
// We must manually trigger a reload to ensure the new navigation attempts a retry.
166+
(underlyingRes as unknown as {reload?: () => boolean}).reload?.();
167+
}
168+
});
169+
134170
route._futureSnapshot.resources = currentResources;
135-
prohibitBlockingResources(route, currentResources);
171+
setupBlocking(route, currentResources, blockingResourcePromises, abortSignal);
136172
}
137173

138-
function prohibitBlockingResources(route: ActivatedRoute, resourceResult: ResourceResult) {
174+
function setupBlocking(
175+
route: ActivatedRoute,
176+
resourceResult: ResourceResult,
177+
blockingResourcePromises: Array<Promise<void>>,
178+
abortSignal: AbortSignal,
179+
) {
180+
if (abortSignal.aborted) return;
139181
const childInjector = route._localInjector;
140182
if (!childInjector || !resourceResult) return;
141183

@@ -144,6 +186,47 @@ function prohibitBlockingResources(route: ActivatedRoute, resourceResult: Resour
144186
if (res[BLOCKING_SYMBOL] === false) {
145187
continue;
146188
}
147-
throw new Error('blocking resources not implemented yet');
189+
const promise = new Promise<void>((resolve, reject) => {
190+
const underlyingRes = res[SOURCE_RESOURCE_SYMBOL];
191+
let isDestroyed = false;
192+
let unregisterOnDestroy: (() => void) | undefined;
193+
194+
const cleanup = () => {
195+
isDestroyed = true;
196+
blockingEffect.destroy();
197+
unregisterOnDestroy?.();
198+
abortSignal.removeEventListener('abort', onAbort);
199+
};
200+
201+
const onAbort = () => {
202+
cleanup();
203+
resolve();
204+
};
205+
206+
abortSignal.addEventListener('abort', onAbort, {once: true});
207+
208+
const blockingEffect = effect(
209+
() => {
210+
if (isDestroyed) {
211+
return;
212+
}
213+
const status = underlyingRes.status();
214+
if (status === 'error') {
215+
cleanup();
216+
reject(underlyingRes.error());
217+
} else if (hasValueOrResolved(underlyingRes)) {
218+
cleanup();
219+
resolve();
220+
}
221+
},
222+
{injector: childInjector, manualCleanup: true},
223+
);
224+
225+
unregisterOnDestroy = childInjector.get(DestroyRef).onDestroy(() => {
226+
cleanup();
227+
resolve();
228+
});
229+
});
230+
blockingResourcePromises.push(promise);
148231
}
149232
}

‎packages/router/src/router_resource.ts‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,23 @@ import {
3232
export const BLOCKING_SYMBOL: unique symbol = Symbol(
3333
typeof ngDevMode === 'undefined' || ngDevMode ? '__isBlocking' : '',
3434
);
35+
export const SOURCE_RESOURCE_SYMBOL: unique symbol = Symbol(
36+
typeof ngDevMode === 'undefined' || ngDevMode ? '__sourceResource' : '',
37+
);
38+
39+
/**
40+
* Checks if a resource has a value or has transitioned to a resolved/non-loading status.
41+
*/
42+
export function hasValueOrResolved(res: Resource<unknown>): boolean {
43+
const status = res.status();
44+
return res.hasValue() || (status !== 'loading' && status !== 'reloading');
45+
}
3546

3647
/**
3748
* @internal
3849
*/
3950
export interface InternalRouterResource<T = unknown> extends Resource<T> {
51+
[SOURCE_RESOURCE_SYMBOL]: Resource<T>;
4052
[BLOCKING_SYMBOL]?: boolean;
4153
reload(): boolean;
4254
}
@@ -68,11 +80,9 @@ export function routerResource<T>(source: Resource<T>): Resource<T> & {reload():
6880

6981
const res = resourceFromSnapshots(snapshotSignal) as unknown as InternalRouterResource<T>;
7082

71-
if ((source as unknown as InternalRouterResource<T>)[BLOCKING_SYMBOL] === false) {
72-
res[BLOCKING_SYMBOL] = false;
73-
} else {
74-
res[BLOCKING_SYMBOL] = true;
75-
}
83+
res[SOURCE_RESOURCE_SYMBOL] = source;
84+
res[BLOCKING_SYMBOL] =
85+
(source as unknown as InternalRouterResource<T>)[BLOCKING_SYMBOL] !== false;
7686

7787
if (typeof (source as any).reload === 'function') {
7888
res.reload = function (): boolean {
@@ -155,12 +165,7 @@ function createTransactionalSnapshot<T>(
155165

156166
effect(
157167
() => {
158-
if (
159-
isRollbackRecoveryPending() &&
160-
// TODO(consider): should this be hasValue || status !== loading
161-
// Some stream implementations may retain loading status after first item resolves
162-
!source.isLoading()
163-
) {
168+
if (isRollbackRecoveryPending() && hasValueOrResolved(source)) {
164169
isRollbackRecoveryPending.set(false);
165170
frozenSnapshot.set(null);
166171
}

‎packages/router/test/router_resource_behavior_spec.ts‎

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,21 @@
22
* @license
33
* Copyright Google LLC All Rights Reserved.
44
*
5-
* Use of this source code is governed by an MIT-style license $can be
5+
* Use of this source code is governed by an MIT-style license that can be
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {Component, signal, WritableSignal, resource, ɵpromiseWithResolvers} from '@angular/core';
9+
import {
10+
Component,
11+
computed,
12+
Resource,
13+
ResourceStatus,
14+
Signal,
15+
signal,
16+
WritableSignal,
17+
resource,
18+
ɵpromiseWithResolvers,
19+
} from '@angular/core';
1020
import {TestBed} from '@angular/core/testing';
1121
import {provideRouter, Router, UrlTree} from '@angular/router';
1222
import {RouterTestingHarness} from '@angular/router/testing';
@@ -455,5 +465,48 @@ describe('routerResource behavior tests', () => {
455465
await harness.fixture.whenStable();
456466
expect(wrapped.value()).toBe('updated-2');
457467
});
468+
469+
it('should complete rollback recovery when a resource has a value even while remaining in loading state', async () => {
470+
const valueSignal = signal<string | undefined>('initial');
471+
const hasValueSignal = signal<boolean>(true);
472+
473+
const customResource: Resource<string> = {
474+
value: valueSignal as Signal<string>,
475+
status: signal<ResourceStatus>('loading').asReadonly(),
476+
isLoading: signal(true).asReadonly(),
477+
hasValue: (() => hasValueSignal()) as any,
478+
error: signal<Error | undefined>(undefined).asReadonly(),
479+
snapshot: computed(() => ({
480+
status: 'loading' as const,
481+
value: valueSignal()!,
482+
})),
483+
};
484+
485+
const wrapped = TestBed.runInInjectionContext(() => routerResource(customResource));
486+
expect(wrapped.value()).toBe('initial');
487+
488+
// Start navigation to route2 with a failing guard to trigger rollback
489+
guardPromise2 = Promise.reject(new Error('Navigation failed'));
490+
try {
491+
await harness.navigateByUrl('/route2');
492+
} catch {}
493+
494+
// Reset value and set hasValue to false to simulate recovery fetch starting
495+
valueSignal.set(undefined);
496+
hasValueSignal.set(false);
497+
await timeout();
498+
499+
// Wrapped snapshot should be frozen at 'initial' during recovery loading
500+
expect(wrapped.value()).toBe('initial');
501+
502+
// Resource receives value while isLoading() remains true and status is 'loading'
503+
valueSignal.set('recovered-stream-1');
504+
hasValueSignal.set(true);
505+
await harness.fixture.whenStable();
506+
507+
// Rollback recovery unfreezes because hasValue is true despite isLoading being true
508+
expect(wrapped.value()).toBe('recovered-stream-1');
509+
expect(wrapped.isLoading()).toBe(true);
510+
});
458511
});
459512
});

0 commit comments

Comments
 (0)