Skip to content

feat(zone.js): monkey patches queueMicrotask()#38904

Closed
JiaLiPassion wants to merge 1 commit intoangular:masterfrom
JiaLiPassion:queuemicrotask
Closed

feat(zone.js): monkey patches queueMicrotask()#38904
JiaLiPassion wants to merge 1 commit intoangular:masterfrom
JiaLiPassion:queuemicrotask

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();
  });
});

@JiaLiPassion JiaLiPassion added the area: zones Issues related to zone.js label Sep 19, 2020
@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.

@JiaLiPassion JiaLiPassion force-pushed the queuemicrotask branch 3 times, most recently from 65ef811 to 8b2b9ae Compare September 30, 2020 06:30
@JiaLiPassion JiaLiPassion added the target: major This PR is targeted for the next major release label Sep 30, 2020
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
Contributor

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 Issues related to zone.js 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