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

Fix release regressions. #14281

Merged
merged 2 commits into from
Sep 13, 2016
Merged

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Sep 13, 2016

Fixes #14266.
Fixes #14273.

Note This targets release directly (as HTMLBars is no longer part of master/beta branches).

@rwjblue rwjblue added this to the 2.8.0 milestone Sep 13, 2016
@krisselden
Copy link
Contributor

@rwjblue looks good

@krisselden
Copy link
Contributor

@rwjblue I'm not sure about the || in the condition, why would you ever want to schedule if isDestroying?

@@ -478,7 +478,7 @@ export default Mixin.create({
{ id: 'ember-views.dispatching-modify-property', until: '3.0.0' }
);

if (!this.scheduledRevalidation || this._dispatching) {
if ((!this.scheduledRevalidation && !this.isDestroying) || this._dispatching) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this boolean doesn't seem correct, why would we schedule if isDestroying but we are this._dispatching? seems like isDestroying should just exit early altogether.

@rwjblue
Copy link
Member Author

rwjblue commented Sep 13, 2016

Good point, updated.

Robert Jackson added 2 commits September 13, 2016 14:21
Fixes regression related to `willDestroyElement` causing changes to
subscribed streams. When that happened, the system would trigger
`scheduleRevalidate` on the view (even though it was being destroyed).
By the time that the async rerender occurs the queue of things to
cleanup in `_destroyingSubtreeForView` has been reset to `null` causing
the error reported.

This fix simply avoids queueing up rerenders when the component is being destroyed.
Setting `parentView` to `null` is technically better, but unfortunately
we have not had this reset in place for a while and folks have dependent
keys that include `parentView`. Setting without `Ember.set` triggers the
mandatory setter assertion.

This removes the clearing of `parentView` (for now) so that we can
properly message it being unobservable in the future.
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