Skip to content

refactor(core): internal tracker of pending tasks during initial rendering#49576

Closed
AndrewKushnir wants to merge 1 commit into
angular:mainfrom
AndrewKushnir:pending-tasks-tracker
Closed

refactor(core): internal tracker of pending tasks during initial rendering#49576
AndrewKushnir wants to merge 1 commit into
angular:mainfrom
AndrewKushnir:pending-tasks-tracker

Conversation

@AndrewKushnir

@AndrewKushnir AndrewKushnir commented Mar 24, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

This commit implements a simple tracker of pending tasks. The class allows adding and removing tasks from the set. The class also exposes a promise that gets resolved once the last task is removed.

This tracker is needed to keep track of ongoing processes like Router navigation (and potentially HTTP requests) and acts as a signaling mechanism to SSR and hydration that the application is in the "stable" state and a serialization can be performed.

This class would also act as a future replacement for the ApplicationRef.isStable for zoneless applications.

PR Type

What kind of change does this PR introduce?

  • Refactoring (no functional changes, no api changes)

Does this PR introduce a breaking change?

  • Yes
  • No

@AndrewKushnir AndrewKushnir added state: WIP area: core Issues related to the framework runtime target: major This PR is targeted for the next major release labels Mar 24, 2023
@ngbot ngbot Bot modified the milestone: Backlog Mar 24, 2023
@AndrewKushnir

AndrewKushnir commented Mar 24, 2023

Copy link
Copy Markdown
Contributor Author

This can also be leveraged by the changes in #49546 to avoid setTimeouts and be more zoneless-ready. // cc @alan-agius4

@AndrewKushnir AndrewKushnir force-pushed the pending-tasks-tracker branch from 6216a59 to a30aed1 Compare March 24, 2023 01:27
alfaproject
alfaproject reviewed Mar 24, 2023
View reviewed changes
Comment thread packages/core/src/pending_tasks.ts Outdated
alan-agius4
alan-agius4 reviewed Mar 24, 2023
View reviewed changes
Comment thread packages/core/src/pending_tasks.ts Outdated
Comment thread packages/core/src/pending_tasks.ts Outdated
Comment thread packages/core/src/pending_tasks.ts Outdated
Comment thread packages/core/src/pending_tasks.ts Outdated
@AndrewKushnir AndrewKushnir force-pushed the pending-tasks-tracker branch from a30aed1 to 9272639 Compare March 25, 2023 02:02
@AndrewKushnir AndrewKushnir changed the title refactor(core): internal tracker of pending tasks refactor(core): internal tracker of pending tasks during initial rendering Mar 25, 2023
@AndrewKushnir AndrewKushnir force-pushed the pending-tasks-tracker branch from 9272639 to 7d7b363 Compare March 25, 2023 02:56
@AndrewKushnir

AndrewKushnir commented Mar 27, 2023

Copy link
Copy Markdown
Contributor Author

Exploratory presubmit.

@AndrewKushnir AndrewKushnir force-pushed the pending-tasks-tracker branch 4 times, most recently from 25d8f66 to c4cdc40 Compare March 28, 2023 00:03
@AndrewKushnir AndrewKushnir added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels Mar 28, 2023
@AndrewKushnir AndrewKushnir marked this pull request as ready for review March 28, 2023 00:28
@AndrewKushnir

AndrewKushnir commented Mar 28, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Update: after a conversation with @alan-agius4, we decided that PR #49546 will land as it (without using the class from this PR). In conjunction with this PR, this should be sufficient for applications that use ZoneJS: both routing (including lazy-loaded routes) and data fetches (via @angular/common/http) should be handled correctly during SSR (serialization of the application will happen at the right time). There will be a separate effort (post v16) to support SSR for Zone-less applications.

@AndrewKushnir AndrewKushnir mentioned this pull request Mar 28, 2023
Closed
3 tasks
@AndrewKushnir AndrewKushnir added the action: global presubmit The PR is in need of a google3 global presubmit label Mar 28, 2023
@AndrewKushnir

AndrewKushnir commented Mar 28, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Global Presubmit.

alan-agius4
alan-agius4 approved these changes Mar 28, 2023
View reviewed changes

@alan-agius4 alan-agius4 left a comment

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.

LGTM

jessicajaniuk
jessicajaniuk approved these changes Mar 28, 2023
View reviewed changes

@jessicajaniuk jessicajaniuk left a comment

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.

LGTM! I like how clean the pending tasks implementation is.

reviewed-for: fw-core, fw-platform-server, public-api

@AndrewKushnir
refactor(core): internal tracker of pending tasks during initial rend…
0cddae6 
…ering

This commit implements a simple tracker of the pending tasks during initial rendering. The class allows adding and removing tasks from the set. The class also exposes a promise that gets resolved once the last task is removed.

This tracker is needed to keep track of ongoing processes like Router navigation (and potentially HTTP requests) and acts as a signaling mechanism to SSR and hydration that the application is in the "stable" state and a serialization can be performed.

This class would also act as a future replacement for the `ApplicationRef.isStable` for zoneless applications.
@AndrewKushnir AndrewKushnir force-pushed the pending-tasks-tracker branch from c4cdc40 to 0cddae6 Compare March 28, 2023 16:46
@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: review The PR is still awaiting reviews from at least one requested reviewer action: global presubmit The PR is in need of a google3 global presubmit labels Mar 28, 2023
@AndrewKushnir

AndrewKushnir commented Mar 28, 2023

Copy link
Copy Markdown
Contributor Author

Caretaker note: TGP is "green", the PR is ready for merge after @atscott's review (if there are no blockers identified during the review).

@atscott

atscott commented Mar 28, 2023

Copy link
Copy Markdown
Contributor

This PR was merged into the repository by commit 478c5ac.

@atscott atscott closed this in 478c5ac Mar 28, 2023
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Apr 28, 2023

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Apr 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@atscott atscott atscott approved these changes
@alan-agius4 alan-agius4 alan-agius4 approved these changes
+2 more reviewers
@alfaproject alfaproject alfaproject left review comments
@jessicajaniuk jessicajaniuk jessicajaniuk approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: major This PR is targeted for the next major release

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

5 participants

@AndrewKushnir @atscott @alfaproject @alan-agius4 @jessicajaniuk