Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

fix(core): don't update taskCounts of setInterval #999

Closed
wants to merge 1 commit into from

Conversation

JiaLiPassion
Copy link
Collaborator

@JiaLiPassion JiaLiPassion commented Jan 25, 2018

fix angular/angular#20970 (comment)

don't update taskCount or trigger hasTask for setInterval.
otherwise the zone will always be unstable.

update

now the CI failed because new version jasmine complains, I have pinned the version of jasmine in #994.

@@ -60,10 +60,15 @@ describe('setInterval', function() {
}, null, null, 'unit-test');
});

it('should not cancel the task after invoke the setInterval callback', (done) => {
it('setInterval should not update task count to trigger onHasTask', (done: Function) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is right. Why should setInterval be exempt from tasks?

Copy link
Collaborator Author

@JiaLiPassion JiaLiPassion Feb 10, 2018

Choose a reason for hiding this comment

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

@mhevery
I just re-think it, you are right, we should not exempt interval from tasks, the interval should behave like

  • call updateTaskCount +1 and trigger onHasTask when scheduleTask.
  • will ** not ** call updateTaskCount and trigger onHasTask after runTask.
  • will call updateTaskCount -1 and trigger onHasTask when cancelTask.

so if some interval task is active, zone will not be stable, but it is the correct behavior.

@JiaLiPassion
Copy link
Collaborator Author

@mhevery
I just re-think it, you are right, we should not exempt interval from tasks, the interval should behave like

call updateTaskCount +1 and trigger onHasTask when scheduleTask.
will ** not ** call updateTaskCount and trigger onHasTask after runTask.
will call updateTaskCount -1 and trigger onHasTask when cancelTask.
so if some interval task is active, zone will not be stable, but it is the correct behavior.

I will close this PR, the current behavior is correct. thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(service-worker): does not start/install (zone not stabilizing)
3 participants