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): monkey patches queueMicrotask() #38904

Closed
wants to merge 1 commit into from

Conversation

JiaLiPassion
Copy link
Contributor

Close #38863

Monkey patches queueMicrotask() API, so the callback runs in the zone
when scheduled, and also the task is run as microTask.

Zone.current.fork({
  name: 'queueMicrotask',
  onScheduleTask: (delegate: ZoneDelegate, curr: Zone, target: Zone, task: Task) => {
    logs.push(task.type);
    logs.push(task.source);
    return delegate.scheduleTask(target, task);
  }
}).run(() => {
    queueMicrotask(() => {
      expect(logs).toEqual(['microTask', 'queueMicrotask']);
      expect(Zone.current.name).toEqual('queueMicrotask');
      done();
  });
});

@ngbot ngbot bot added this to the needsTriage milestone Sep 19, 2020
Copy link
Contributor

@mhevery mhevery left a comment

Choose a reason for hiding this comment

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

LGTM, but please incorporate these changes.

packages/zone.js/lib/zone.configurations.api.ts Outdated Show resolved Hide resolved
packages/zone.js/lib/zone.configurations.api.ts Outdated Show resolved Hide resolved
packages/zone.js/lib/zone.configurations.api.ts Outdated Show resolved Hide resolved
packages/zone.js/lib/zone.configurations.api.ts Outdated Show resolved Hide resolved
@JiaLiPassion JiaLiPassion force-pushed the queuemicrotask branch 3 times, most recently from 65ef811 to 8b2b9ae Compare September 30, 2020 06:30
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

LGTM (based on the comment here)

Reviewed-for: size-tracking

@pullapprove pullapprove bot requested a review from atscott October 1, 2020 17:10
Copy link
Contributor

@atscott atscott 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: size-tracking

@petebacondarwin petebacondarwin removed their request for review October 2, 2020 13:17
@jelbourn jelbourn removed their request for review October 7, 2020 21:57
@mhevery mhevery added the action: merge The PR is ready for merge by the caretaker label Oct 19, 2020
@mhevery
Copy link
Contributor

mhevery commented Oct 19, 2020

presubmit

@mhevery mhevery self-assigned this Oct 19, 2020
Close angular#38863

Monkey patches `queueMicrotask()` API, so the callback runs in the zone
when scheduled, and also the task is run as `microTask`.

```
Zone.current.fork({
  name: 'queueMicrotask',
  onScheduleTask: (delegate: ZoneDelegate, curr: Zone, target: Zone, task: Task) => {
    logs.push(task.type);
    logs.push(task.source);
    return delegate.scheduleTask(target, task);
  }
}).run(() => {
    queueMicrotask(() => {
      expect(logs).toEqual(['microTask', 'queueMicrotask']);
      expect(Zone.current.name).toEqual('queueMicrotask');
      done();
  });
});

```
@jelbourn
Copy link
Member

jelbourn commented Nov 4, 2020

@JiaLiPassion is this a breaking change? If not it could be target: minor

@JiaLiPassion JiaLiPassion added target: minor This PR is targeted for the next minor release and removed target: major This PR is targeted for the next major release labels Nov 4, 2020
@JiaLiPassion
Copy link
Contributor Author

@jelbourn , got it, updated, thanks.

@mhevery mhevery removed the request for review from alxhub November 5, 2020 19:22
@mhevery mhevery closed this in 27358eb Nov 5, 2020
@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 Dec 6, 2020
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 cla: yes target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

queueMicrotask() not patched in zone.js
7 participants