diff --git a/packages/router/src/router_resource.ts b/packages/router/src/router_resource.ts index 2a3d74484a3..b315c6932e2 100644 --- a/packages/router/src/router_resource.ts +++ b/packages/router/src/router_resource.ts @@ -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): boolean { - const status = res.status(); - return res.hasValue() || (status !== 'loading' && status !== 'reloading'); -} - /** * @internal */ @@ -169,7 +161,7 @@ function createTransactionalSnapshot( effect( () => { - if (isRollbackRecoveryPending() && hasValueOrResolved(source)) { + if (isRollbackRecoveryPending() && !source.isLoading()) { isRollbackRecoveryPending.set(false); frozenSnapshot.set(null); } diff --git a/packages/router/test/router_resource_behavior_spec.ts b/packages/router/test/router_resource_behavior_spec.ts index a4852c8e929..52a2b69abb1 100644 --- a/packages/router/test/router_resource_behavior_spec.ts +++ b/packages/router/test/router_resource_behavior_spec.ts @@ -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('initial'); + const statusSignal = signal('resolved'); + const isLoadingSignal = signal(false); const hasValueSignal = signal(true); const customResource: Resource = { value: valueSignal as Signal, - status: signal('loading').asReadonly(), - isLoading: signal(true).asReadonly(), + status: statusSignal.asReadonly(), + isLoading: isLoadingSignal.asReadonly(), hasValue: (() => hasValueSignal()) as any, error: signal(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); }); }); }); diff --git a/packages/router/test/router_resource_spec.ts b/packages/router/test/router_resource_spec.ts index c1773749f4c..f3ec5dba897 100644 --- a/packages/router/test/router_resource_spec.ts +++ b/packages/router/test/router_resource_spec.ts @@ -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(); + + 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(); + + 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;