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 release] Consumes tags after fully getting values #18668

Merged
merged 1 commit into from
Jan 16, 2020

Conversation

pzuraq
Copy link
Contributor

@pzuraq pzuraq commented Jan 9, 2020

Currently get() consumes some tags before getting the values,
and getting the values itself can cause tag changes and notifications.
In some cases, this is invalid, because the value that is being notified
hasn't actually been consumed yet (it is in the process of being
consumed. More generally, consumption should always occur immediately
after the relevant calculation - in this case, the getter.

Currently get() consumes some tags before getting the values,
and getting the values itself can cause tag changes and notifications.
In some cases, this is invalid, because the value that is being notified
hasn't actually been consumed yet (it is in the process of being
consumed. More generally, consumption should always occur immediately
_after_ the relevant calculation - in this case, the getter.
@lifeart
Copy link
Contributor

lifeart commented Jan 12, 2020

looks like machty/ember-concurrency#337 may be related

@makepanic
Copy link
Contributor

Thanks, this seems to solve emberjs/data#6943 .

Is there a plan on when this will be merged?

@@ -402,5 +402,25 @@ moduleFor(
});
}, /You attempted to update `foo` on `EmberObject`, but it had already been used previously in the same computation/);
}

['@test get() does not entangle in the autotracking stack until after retrieving the value'](
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain this test? How did it fail before? How does it confirm what it says it is confirming?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before, get would consume the tag for the property immediately, then actually get the value of the property. If the property signaled a change via notifyPropertyChange in this time, it would throw the backtracking assertion. This is the fundamental problem that led to the bug in Ember Data.

After, we consume all tags for the first time after getting resolved values, whatever they end up being, which means it is safe to signal property changes while calculating the value.

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.

4 participants