-
Notifications
You must be signed in to change notification settings - Fork 27.2k
refactor(router): Create transactional router resource #69490
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| /** | ||
| * @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 { | ||
| inject, | ||
| Injector, | ||
| Resource, | ||
| resourceFromSnapshots, | ||
| Signal, | ||
| signal, | ||
| DestroyRef, | ||
| ResourceSnapshot, | ||
| effect, | ||
| computed, | ||
| WritableResource, | ||
| assertInInjectionContext, | ||
| } from '@angular/core'; | ||
| import {Router} from './router'; | ||
| import { | ||
| NavigationStart, | ||
| NavigationEnd, | ||
| NavigationCancel, | ||
| NavigationError, | ||
| NavigationSkipped, | ||
| NavigationCancellationCode, | ||
| } from './events'; | ||
|
|
||
| /** | ||
| * Wraps a Resource to make it cooperative with the Angular Router, freezing its state | ||
| * during navigation transitions and handling rollback recovery. | ||
| */ | ||
| export function routerResource<T>(source: Resource<T>): Resource<T> & {reload(): boolean} { | ||
| ngDevMode && assertInInjectionContext(routerResource); | ||
| const injector = inject(Injector); | ||
| const router = injector.get(Router); | ||
|
|
||
| const {snapshot: snapshotSignal, frozenSnapshot} = createTransactionalSnapshot( | ||
| source, | ||
| router, | ||
| injector, | ||
| ); | ||
|
|
||
| const res = resourceFromSnapshots(snapshotSignal) as Resource<T> & {reload(): boolean}; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Side conversation, this reminds me of #67124. We might want to have such feature in the public api.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, it's a bit of that which seems to also be a consequence of |
||
|
|
||
| res.reload = function (): boolean { | ||
| if (frozenSnapshot() !== null) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you leave a comment explaining why a non-null frozen snapshot means we can't/don't reload? I'm guessing it's this:
|
||
| return false; | ||
| } | ||
| return (source as WritableResource<T>).reload?.() ?? false; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the if (isWritableResource(source)) {
res.reload = () => frozenSnapshot() !== null && source.reload();
} else {
res.reload = () => false;
}Since the always false branch is stateless you could also avoid the closure allocation (not sure how commonly a resource won't be writable though). |
||
| }; | ||
|
|
||
| return res; | ||
| } | ||
|
|
||
| /** | ||
| * Creates a signal that tracks the resource snapshot and handles transactional behavior | ||
| * (freezing during navigation and rollback recovery). | ||
| */ | ||
| function createTransactionalSnapshot<T>( | ||
| source: Resource<T>, | ||
| router: Router, | ||
| injector: Injector, | ||
| ): { | ||
| snapshot: Signal<ResourceSnapshot<T>>; | ||
| frozenSnapshot: Signal<ResourceSnapshot<T> | null>; | ||
| } { | ||
| // Holds a snapshot of the resource to keep the UI masked (frozen) during pending navigations | ||
| // or while recovering from a cancelled navigation. | ||
| const frozenSnapshot = signal<ResourceSnapshot<T> | null>(null); | ||
|
|
||
| // Tracks whether we are in a recovery phase after a cancelled navigation. | ||
| // The intended behavior is that on cancellation, the router reverts to the previous state. | ||
| // This reversion might trigger a new load of the previous state because the signal dependencies | ||
| // changed. If we were to release the frozen resource state immediately, the user would see a loading state | ||
| // for data they were just looking at. To avoid this "loading flash", we retain the frozen | ||
| // value (via frozenSnapshot) during this recovery load/reload until the resource settles. | ||
| const isRollbackRecoveryPending = signal(false); | ||
|
|
||
| const sub = router.events.subscribe((e) => { | ||
| if (e instanceof NavigationStart) { | ||
| isRollbackRecoveryPending.set(false); | ||
|
|
||
| if (frozenSnapshot() === null) { | ||
| // Freeze the snapshot at the start of navigation to keep the UI stable. | ||
| frozenSnapshot.set(source.snapshot()); | ||
| } | ||
| } else if (e instanceof NavigationEnd || e instanceof NavigationSkipped) { | ||
| // Navigation succeeded or was skipped, so we can unfreeze and use the live state. | ||
| frozenSnapshot.set(null); | ||
| isRollbackRecoveryPending.set(false); | ||
| } else if (e instanceof NavigationCancel || e instanceof NavigationError) { | ||
| const isRollback = | ||
| e instanceof NavigationError || | ||
| (e instanceof NavigationCancel && | ||
| e.code !== NavigationCancellationCode.SupersededByNewNavigation && | ||
| e.code !== NavigationCancellationCode.Redirect); | ||
|
|
||
| if (!isRollback) return; | ||
|
|
||
| const frozen = frozenSnapshot(); | ||
|
|
||
| // Because `rollbackState` runs synchronously immediately prior to `NavigationCancel` (for true rollbacks), | ||
| // the underlying resource parameters have already reverted. | ||
| // If those parameters triggered a reload, `isLoading` will synchronously remain true here. | ||
| if (frozen?.status === 'resolved' || frozen?.status === 'reloading') { | ||
| // We were in a valid state, so keep the UI frozen while we wait for the recovery load to complete. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And when that recovery completes |
||
| isRollbackRecoveryPending.set(true); | ||
| } else { | ||
| // We were not in a valid state, so we can't recover. Unfreeze immediately. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably just a knowledge deficit on my part, but what happens (a) to get in this state and (b) to recover from it? |
||
| isRollbackRecoveryPending.set(false); | ||
| frozenSnapshot.set(null); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| injector.get(DestroyRef).onDestroy(() => sub.unsubscribe()); | ||
|
|
||
| effect( | ||
| () => { | ||
| if (isRollbackRecoveryPending() && !source.isLoading()) { | ||
| isRollbackRecoveryPending.set(false); | ||
| frozenSnapshot.set(null); | ||
| } | ||
| }, | ||
| {injector}, | ||
| ); | ||
|
|
||
| return { | ||
| snapshot: computed(() => frozenSnapshot() ?? source.snapshot()), | ||
| frozenSnapshot, | ||
| }; | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.