Skip to content

refactor(router): add support for blocking router resources - #70293

Open
atscott wants to merge 2 commits into
angular:mainfrom
atscott:resourcestackblocking
Open

refactor(router): add support for blocking router resources#70293
atscott wants to merge 2 commits into
angular:mainfrom
atscott:resourcestackblocking

Conversation

@atscott

@atscott atscott commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Extends router resource integration to support blocking resources during navigation transitions.

Resources declared in a route's resources function are blocking by default unless explicitly marked with nonBlocking(). Key behaviors include:

  • During the navigation transition, the router awaits the resolution of all blocking resources across target routes before proceeding to activation and completing navigation.
  • Blocking status is tracked via an effect on the underlying resource: if the resource settles with a value, the promise resolves; if the resource status becomes 'error', the promise rejects with the resource error, cancelling navigation and emitting NavigationError.
  • If a reused route is re-navigated to after a resource has errored (e.g., with onSameUrlNavigation: 'reload'), the router triggers .reload() on the underlying resource to retry the failed fetch.
  • All tracking effects are cleaned up upon resolution, rejection, or route injector destruction.
  • Supports both sync and async resources functions (returning Promise<ResourceResult>) as well as rxResource.

@ngbot ngbot Bot added this to the Backlog milestone Aug 19, 2026
@atscott atscott added the target: patch This PR is targeted for the next patch release label Aug 19, 2026
@atscott
atscott force-pushed the resourcestackblocking branch 9 times, most recently from d685fd5 to 08c24b3 Compare August 20, 2026 16:30
Extends router resource integration to support blocking resources during navigation transitions.

Resources declared in a route's `resources` function are blocking by default unless explicitly marked with `nonBlocking()`. Key behaviors include:
- During the navigation transition, the router awaits the resolution of all blocking resources across target routes before proceeding to activation and completing navigation.
- Blocking status is tracked via an effect on the underlying resource: if the resource settles with a value, the promise resolves; if the resource status becomes 'error', the promise rejects with the resource error, cancelling navigation and emitting `NavigationError`.
- If a reused route is re-navigated to after a resource has errored (e.g., with `onSameUrlNavigation: 'reload'`), the router triggers `.reload()` on the underlying resource to retry the failed fetch.
- All tracking effects are cleaned up upon resolution, rejection, or route injector destruction.
- Supports both sync and async `resources` functions (returning `Promise<ResourceResult>`) as well as `rxResource`.
@atscott
atscott force-pushed the resourcestackblocking branch from 08c24b3 to 93857a5 Compare August 20, 2026 16:35
@atscott
atscott marked this pull request as ready for review August 20, 2026 16:40
@atscott
atscott requested a review from leonsenft August 20, 2026 16:41

return Promise.all(resourceSetupPromises);
// TODO: wait for blocking resources
return Promise.all(resourceSetupPromises).then(() => Promise.all(blockingResourcePromises));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need all setup promises to resolve before we await the blocking resource promises? Can't we just await them all?

return Promise.all([...resourceSetupPromises, ...blockingResourcePromises]);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I nearly made this mistake today when giving it a final look. We have to wait for setup promises because only once those are resolved do we add to the blockingResourcePromises array.

Comment thread packages/router/test/router_resource_behavior_spec.ts
// Wrapped snapshot should be frozen at 'initial' during recovery loading
expect(wrapped.value()).toBe('initial');

// Resource receives value while isLoading() remains true and status is 'loading'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't completely follow this, why would it still be loading if the resource is resolved?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This came from feedback internally. They have a custom streaming resource that has this behavior. It has a value after it receives the first value but stays in the loading state until the underlying rxjs observable is marked completed.

Comment thread packages/router/test/router_resource_spec.ts Outdated
expect(resourceRef.value()).toBe('rx loaded 123');
});

it('should unblock navigation when a resource emits a value even while remaining in loading state', async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the same use case I was confused about in the other test. Why does it remain loading?

@atscott
atscott force-pushed the resourcestackblocking branch from b8fabc9 to fab0f26 Compare August 20, 2026 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: router target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants