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(zone.js): Update to the simpler Async Stack Tagging v2 API #46958

Closed
wants to merge 1 commit into from

Conversation

victorporof
Copy link
Contributor

Signed-off-by: Victor Porof victorporof@chromium.org

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

Chrome DevTools is planning on dropping the experimental flag for the new async stack tagging API and enabling this feature by default on the console object. As part of this, the API has been improved to be more robust. More details in the chromium bug and in the V8 implementation.

Does this PR introduce a breaking change?

  • Yes
  • No

@pullapprove pullapprove bot requested a review from JiaLiPassion July 25, 2022 09:08
@victorporof
Copy link
Contributor Author

@JiaLiPassion @mgechev PTAL!

task.asyncId = this.scheduleAsyncTask(
task.source || task.type, task.data?.isPeriodic || task.type === 'eventTask');
onScheduleTask(delegate: ZoneDelegate, _current: Zone, target: Zone, task: Task): Task {
task.consoleTask = this.scheduleTask(`Zone - ${task.source || task.type}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

In the previous version, scheduleAsyncTask has the 2nd parameter which tells console the task is period or not,

  1. if it is not period task, then finishAsyncTask can only be called once.
  2. if it is period task such as eventTask or setInterval, finishAsyncTask can be called multiple times.

So in the v2 API, this spec has been changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, in the new API, tasks can always be recurring (aka, they can run multiple times). Consumers of the API don't need to specify this beforehand, nor do they need to cancel the tasks anymore. More details here.

@ngbot ngbot bot added this to the Backlog milestone Jul 25, 2022
Signed-off-by: Victor Porof <victorporof@chromium.org>
@victorporof
Copy link
Contributor Author

Last minute change: we renamed console.scheduleTask to console.createTask. Updated the PR to reflect that.

@JiaLiPassion
Copy link
Contributor

@victorporof , thank you for the explaination, LGTM.

@JiaLiPassion JiaLiPassion added action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release labels Jul 26, 2022
@AndrewKushnir AndrewKushnir added the action: global presubmit The PR is in need of a google3 global presubmit label Aug 1, 2022
@AndrewKushnir
Copy link
Contributor

@victorporof could you please run a TGP with those changes to make sure there are no failures in g3? Thank you.

@victorporof
Copy link
Contributor Author

@AndrewKushnir I'm not sure what that means :) What button should I press and where can I find it? 😄

@AndrewKushnir
Copy link
Contributor

@victorporof no worries, I'll take care of it and let you know if there are any failures.

@AndrewKushnir AndrewKushnir self-assigned this Aug 3, 2022
@AndrewKushnir
Copy link
Contributor

Global Presubmit.

@victorporof
Copy link
Contributor Author

Global Presubmit.

Thank you @AndrewKushnir! I see a few failures there, are they related to this PR?

@AndrewKushnir AndrewKushnir removed the action: global presubmit The PR is in need of a google3 global presubmit label Aug 4, 2022
@AndrewKushnir
Copy link
Contributor

@victorporof test failures turned out to be flakes, so this PR is now ready for merge. Thank you.

@AndrewKushnir AndrewKushnir added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Aug 4, 2022
@AndrewKushnir
Copy link
Contributor

Merge-assistance: LGTM from Jia should be enough to proceed with this change. TGP is "green".

@dylhunn
Copy link
Contributor

dylhunn commented Aug 4, 2022

This PR was merged into the repository by commit f23232f.

@dylhunn dylhunn closed this in f23232f Aug 4, 2022
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Aug 23, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [zone.js](https://github.com/angular/angular) ([changelog](https://github.com/angular/angular/blob/master/packages/zone.js/CHANGELOG.md)) | dependencies | patch | [`0.11.7` -> `0.11.8`](https://renovatebot.com/diffs/npm/zone.js/0.11.7/0.11.8) |

---

### Release Notes

<details>
<summary>angular/angular</summary>

### [`v0.11.8`](https://github.com/angular/angular/blob/HEAD/packages/zone.js/CHANGELOG.md#&#8203;0118-httpsgithubcomangularangularcomparezonejs-0117zonejs-0118-2022-08-08)

[Compare Source](angular/angular@zone.js-0.11.7...zone.js-0.11.8)

##### Features

-   **zone.js:** Update to the simpler Async Stack Tagging v2 API ([#&#8203;46958](angular/angular#46958)) ([f23232f](angular/angular@f23232f))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xNTYuMCIsInVwZGF0ZWRJblZlciI6IjMyLjE1OS4xIn0=-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1504
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@angular-automatic-lock-bot
Copy link

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 Sep 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: zones merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note 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

6 participants