mirror of
https://github.com/angular/angular.git
synced 2026-09-14 13:54:52 +08:00
fix(router): maintain frozen state on rollback until resource loading completes
Previously, rollback recovery waited on hasValueOrResolved(), which checked whether the resource already had a value. This caused resources with values (e.g. defaultValue, existing values from prior navigations, or streamed emissions) to unfreeze prematurely while still in a loading state during rollback. This change updates the rollback recovery check in the transactional snapshot effect to be solely determined by !source.isLoading().
This commit is contained in:
@@ -40,14 +40,6 @@ export const SOURCE_RESOURCE_SYMBOL: unique symbol = Symbol(
|
||||
typeof ngDevMode === 'undefined' || ngDevMode ? '__sourceResource' : '',
|
||||
);
|
||||
|
||||
/**
|
||||
* Checks if a resource has a value or has transitioned to a resolved/non-loading status.
|
||||
*/
|
||||
export function hasValueOrResolved(res: Resource<unknown>): boolean {
|
||||
const status = res.status();
|
||||
return res.hasValue() || (status !== 'loading' && status !== 'reloading');
|
||||
}
|
||||
|
||||
/**
|
||||
* @internal
|
||||
*/
|
||||
@@ -169,7 +161,7 @@ function createTransactionalSnapshot<T>(
|
||||
|
||||
effect(
|
||||
() => {
|
||||
if (isRollbackRecoveryPending() && hasValueOrResolved(source)) {
|
||||
if (isRollbackRecoveryPending() && !source.isLoading()) {
|
||||
isRollbackRecoveryPending.set(false);
|
||||
frozenSnapshot.set(null);
|
||||
}
|
||||
|
||||
@@ -466,24 +466,30 @@ describe('routerResource behavior tests', () => {
|
||||
expect(wrapped.value()).toBe('updated-2');
|
||||
});
|
||||
|
||||
it('should complete rollback recovery when a resource has a value even while remaining in loading state', async () => {
|
||||
it('should maintain frozen state during rollback recovery until loading completes', async () => {
|
||||
const valueSignal = signal<string | undefined>('initial');
|
||||
const statusSignal = signal<ResourceStatus>('resolved');
|
||||
const isLoadingSignal = signal<boolean>(false);
|
||||
const hasValueSignal = signal<boolean>(true);
|
||||
|
||||
const customResource: Resource<string> = {
|
||||
value: valueSignal as Signal<string>,
|
||||
status: signal<ResourceStatus>('loading').asReadonly(),
|
||||
isLoading: signal(true).asReadonly(),
|
||||
status: statusSignal.asReadonly(),
|
||||
isLoading: isLoadingSignal.asReadonly(),
|
||||
hasValue: (() => hasValueSignal()) as any,
|
||||
error: signal<Error | undefined>(undefined).asReadonly(),
|
||||
snapshot: computed(() => ({
|
||||
status: 'loading' as const,
|
||||
value: valueSignal()!,
|
||||
})),
|
||||
snapshot: computed(
|
||||
() =>
|
||||
({
|
||||
status: statusSignal(),
|
||||
value: valueSignal()!,
|
||||
}) as any,
|
||||
),
|
||||
};
|
||||
|
||||
const wrapped = TestBed.runInInjectionContext(() => routerResource(customResource));
|
||||
expect(wrapped.value()).toBe('initial');
|
||||
expect(wrapped.isLoading()).toBe(false);
|
||||
|
||||
// Start navigation to route2 with a failing guard to trigger rollback
|
||||
guardPromise2 = Promise.reject(new Error('Navigation failed'));
|
||||
@@ -492,21 +498,33 @@ describe('routerResource behavior tests', () => {
|
||||
} catch {}
|
||||
|
||||
// Reset value and set hasValue to false to simulate recovery fetch starting
|
||||
statusSignal.set('loading');
|
||||
isLoadingSignal.set(true);
|
||||
valueSignal.set(undefined);
|
||||
hasValueSignal.set(false);
|
||||
await timeout();
|
||||
|
||||
// Wrapped snapshot should be frozen at 'initial' during recovery loading
|
||||
expect(wrapped.value()).toBe('initial');
|
||||
expect(wrapped.isLoading()).toBe(false);
|
||||
|
||||
// Resource receives value while isLoading() remains true and status is 'loading'
|
||||
valueSignal.set('recovered-stream-1');
|
||||
hasValueSignal.set(true);
|
||||
await timeout();
|
||||
|
||||
// Rollback recovery remains frozen because isLoading is still true
|
||||
expect(wrapped.value()).toBe('initial');
|
||||
expect(wrapped.isLoading()).toBe(false);
|
||||
|
||||
// Loading completes
|
||||
statusSignal.set('resolved');
|
||||
isLoadingSignal.set(false);
|
||||
await harness.fixture.whenStable();
|
||||
|
||||
// Rollback recovery unfreezes because hasValue is true despite isLoading being true
|
||||
// Rollback recovery unfreezes because loading is complete
|
||||
expect(wrapped.value()).toBe('recovered-stream-1');
|
||||
expect(wrapped.isLoading()).toBe(true);
|
||||
expect(wrapped.isLoading()).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -619,6 +619,62 @@ describe('Router resources integration', () => {
|
||||
expect(resourceRef.isLoading()).toBe(false);
|
||||
});
|
||||
|
||||
it('should maintain frozen state on rollback when navigation fails until rollback reload completes', async () => {
|
||||
let loaderDeferred = promiseWithResolvers<string>();
|
||||
|
||||
const {harness, router} = await setupRouter([
|
||||
{
|
||||
path: 'test/:id',
|
||||
component: TargetCmp,
|
||||
resources: (ctx) => ({
|
||||
data: resource({
|
||||
defaultValue: 'default-value',
|
||||
params: () => ctx.params(),
|
||||
loader: async ({params}) => {
|
||||
if (params['id'] === '2') {
|
||||
throw new Error('Failed navigation');
|
||||
}
|
||||
return loaderDeferred.promise;
|
||||
},
|
||||
}),
|
||||
}),
|
||||
},
|
||||
]);
|
||||
|
||||
// Initial navigation to /test/1
|
||||
const nav1 = harness.navigateByUrl('/test/1');
|
||||
await timeout();
|
||||
loaderDeferred.resolve('loaded-1');
|
||||
await nav1;
|
||||
await harness.fixture.whenStable();
|
||||
|
||||
const resourceRef = (router.routerState.root.firstChild as ActivatedRouteInternal)
|
||||
?.resources?.['data'] as any;
|
||||
expect(resourceRef.value()).toBe('loaded-1');
|
||||
expect(resourceRef.isLoading()).toBe(false);
|
||||
|
||||
// Prepare a new deferred for the reload of id '1' when /test/2 fails and rolls back to /test/1
|
||||
loaderDeferred = promiseWithResolvers<string>();
|
||||
|
||||
const nav2 = harness.navigateByUrl('/test/2').catch(() => {});
|
||||
await nav2;
|
||||
await timeout();
|
||||
|
||||
// The rollback reload for /test/1 is pending (loaderDeferred is not resolved yet).
|
||||
// The resource should remain frozen: value is 'loaded-1', isLoading is false, status is 'resolved'
|
||||
expect(resourceRef.isLoading()).toBe(false);
|
||||
expect(resourceRef.value()).toBe('loaded-1');
|
||||
expect(resourceRef.status()).toBe('resolved');
|
||||
|
||||
// Now resolve the rollback reload
|
||||
loaderDeferred.resolve('reloaded-1');
|
||||
await harness.fixture.whenStable();
|
||||
|
||||
// After settling, resource is unfrozen and has the new value
|
||||
expect(resourceRef.isLoading()).toBe(false);
|
||||
expect(resourceRef.value()).toBe('reloaded-1');
|
||||
});
|
||||
|
||||
it('should abort previous request via AbortSignal when a new navigation comes in', async () => {
|
||||
const deferred = promiseWithResolvers<{name: string}>();
|
||||
let aborted = false;
|
||||
|
||||
Reference in New Issue
Block a user