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

[BUGFIX lts-2-8] Allow canceling items queued by run.schedule. #14550

Merged
merged 1 commit into from
Oct 30, 2016

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Oct 30, 2016

All of the scheduling related Ember.run.* methods return the backburner timer cancelation token with the exception of Ember.run.schedule. (this includes run.next, run.later, run.debounce, run.throttle, and run.scheduleOnce).

This seems like a fairly glaring mistake (and makes it impossible to avoid if (this.isDestroying) { return; } guards when using things like run.schedule('afterRender'), so I have marked it as BUGFIX lts-2-8. If others disagree with the severity here, I can change to a simple [BUGFIX beta].

All of the scheduling related `Ember.run.*` methods return the
backburner timer cancelation token with the exception of
`Ember.run.schedule`. (this includes `run.next`, `run.later`,
`run.debounce`, `run.throttle`, and `run.scheduleOnce`).

This seems like a fairly glaring mistake (and makes it impossible to
avoid `if (this.isDestroying) { return; }` guards when using things like
`run.schedule('afterRender'`), so I have marked it as `BUGFIX lts-2-8`.
If others disagree with the severity here, I can change to a simple
`[BUGFIX beta]`.
@stefanpenner
Copy link
Member

stefanpenner commented Oct 30, 2016

apparently once BackburnerJS/backburner.js#161 may not be cancellable, but schedules should be. But this PR doesn't talk about once or scheduleOnce

BackburnerJS/backburner.js#161 (comment)

@stefanpenner
Copy link
Member

if we are encouraging people to cancel, we may want improve how we cancel BackburnerJS/backburner.js#177

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

Successfully merging this pull request may close these issues.

2 participants