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

fix(timer): setInterval should not auto cancel after callback invoked, fix rxjs version to pass CI #935

Merged
merged 3 commits into from
Dec 27, 2017

Conversation

JiaLiPassion
Copy link
Collaborator

@JiaLiPassion JiaLiPassion commented Oct 21, 2017

fix #934.
setInterval will auto clear timerId after first time invoked.
the issue was imported several versions ago, but it doesn't become a problem because we
return ZoneTask from setInterval before zone.js 0.8.17, from zone.js 0.8.18 , setInterval will return the original timerId, so when we call clearInterval the timerId have been cleared, which will cause zone.js onHasTask not be called, the setInterval task was always there.
And will cause ngZone not stable.

Also fix rxjs version to 5.4.2 to avoid karam error, I will continue to figure out how to resolve karam test error with rxjs 5.5.0.
The issue is here, and it seems it will be fixed in rxjs 5.5.1
ReactiveX/rxjs#2971

@JiaLiPassion JiaLiPassion changed the title fix(timer): setInterval should not auto cancel after callback invoked fix(timer): setInterval should not auto cancel after callback invoked, fix rxjs version to pass CI Oct 27, 2017
Copy link

@adrienverge adrienverge left a comment

Choose a reason for hiding this comment

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

Hi @JiaLiPassion,

I have tested this PR and I confirm it solves the setInterval() / clearInterval() problem (issues #934 and #945)!

Thanks a lot 👍

Do we need to wait for rxjs to publish a new version to merge this, or not?

@JiaLiPassion
Copy link
Collaborator Author

@adrienverge, thank you for confirm, we don't need to wait for rxjs new version, because zone.js doesn't depends on rxjs, but I am not sure when this will be merged.

@sarunint
Copy link

sarunint commented Dec 3, 2017

Since rxjs issue mentioned is resolved, shouldn't you change the rxjs version to ^5.4.2 again?

@JiaLiPassion
Copy link
Collaborator Author

@sarunint , yes, I will update the package.json.

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.

zone.js 0.8.18 setInterval/clearInterval
5 participants