Skip to content
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

feat(core): Add async run method on ExperimentalPendingTasks #56546

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Jun 21, 2024

This helper method is simply a convenience function that reduces some boilerplate with manually adding and removing a task around some asynchronous function. There is a temporary decision here to not return the result of the async function to discourage uses that would allow the application to become stable before making the necessary updates to the state, i.e.

const result = await pendingTasks.run(() => queueMicrotask(() => 'value'));
this.state.set(result);

In the above example, the pending task is removed before the state is set. If this was the last task, the application will become stable. Because we respond to the stability synchronously, this would cause serialization before the state is updated. There are several potential solutions, including:

  • Not removing the task synchronously from the internal PendingTasks service
  • Not setting stability on ApplicationRef synchronously when the last task is removed
  • Not responding to whenStable synchronously in SSR (and fixture.whenStable?)

As things stand today, we need to instead write the APIs which use pending tasks in a way that encourages using the pending tasks in a way that will work with the synchronous stability when the last task is removed.

@atscott atscott added target: minor This PR is targeted for the next minor release core: zoneless Issues related to running Angular without zone.js labels Jun 21, 2024
@angular-robot angular-robot bot added detected: feature PR contains a feature commit area: core Issues related to the framework runtime labels Jun 21, 2024
@ngbot ngbot bot added this to the Backlog milestone Jun 21, 2024
This helper method is simply a convenience function that reduces some
boilerplate with manually adding and removing a task around some
asynchronous function. There is a temporary decision here to _not_
return the result of the async function to discourage uses that would
allow the application to become stable before making the necessary
updates to the state, i.e.

```
const result = await pendingTasks.run(() => queueMicrotask(() => 'value'));
this.state.set(result);
```

In the above example, the pending task is removed before the state is
set. If this was the last task, the application will become stable.
Because we respond to the stability synchronously, this would cause
serialization before the state is updated. There are several potential
solutions, including:

* Not removing the task synchronously from the internal `PendingTasks` service
* Not setting stability on ApplicationRef synchronously when the last task is removed
* Not responding to `whenStable` synchronously in SSR (and fixture.whenStable?)

As things stand today, we need to instead write the APIs which use
pending tasks in a way that encourages using the pending tasks in a way
that will work with the synchronous stability when the last task is
removed.
Copy link
Contributor

@thePunderWoman thePunderWoman left a comment

Choose a reason for hiding this comment

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

reviewed-for: public-api

@pullapprove pullapprove bot requested a review from alxhub June 24, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Issues related to the framework runtime core: zoneless Issues related to running Angular without zone.js detected: feature PR contains a feature commit target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants