Skip to content

Commit

Permalink
[BUGFIX release] Consumes tags after fully getting values
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Chris Garrett committed Jan 10, 2020
1 parent d96d9aa commit 34facf1
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 26 deletions.
45 changes: 19 additions & 26 deletions packages/@ember/-internals/metal/lib/property_get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,31 +105,11 @@ export function get(obj: object, keyName: string): any {
let value: any;

if (isObjectLike) {
let tracking = isTracking();

if (tracking) {
consume(tagForProperty(obj, keyName));
}

if (DEBUG && HAS_NATIVE_PROXY) {
value = getPossibleMandatoryProxyValue(obj, keyName);
} else {
value = obj[keyName];
}

if (tracking) {
// Add the tag of the returned value if it is an array, since arrays
// should always cause updates if they are consumed and then changed
if (Array.isArray(value) || isEmberArray(value)) {
consume(tagForProperty(value, '[]'));
}

// Add the value of the content if the value is a proxy. This is because
// content changes the truthiness/falsiness of the proxy.
if (isProxy(value)) {
consume(tagForProperty(value, 'content'));
}
}
} else {
value = obj[keyName];
}
Expand All @@ -141,18 +121,31 @@ export function get(obj: object, keyName: string): any {
typeof (obj as MaybeHasUnknownProperty).unknownProperty === 'function'
) {
if (DEBUG) {
let ret;

deprecateMutationsInAutotrackingTransaction!(() => {
ret = (obj as MaybeHasUnknownProperty).unknownProperty!(keyName);
value = (obj as MaybeHasUnknownProperty).unknownProperty!(keyName);
});

return ret;
} else {
return (obj as MaybeHasUnknownProperty).unknownProperty!(keyName);
value = (obj as MaybeHasUnknownProperty).unknownProperty!(keyName);
}
}
}

if (isObjectLike && isTracking()) {
consume(tagForProperty(obj, keyName));

// Add the tag of the returned value if it is an array, since arrays
// should always cause updates if they are consumed and then changed
if (Array.isArray(value) || isEmberArray(value)) {
consume(tagForProperty(value, '[]'));
}

// Add the value of the content if the value is a proxy. This is because
// content changes the truthiness/falsiness of the proxy.
if (isProxy(value)) {
consume(tagForProperty(value, 'content'));
}
}

return value;
}

Expand Down
20 changes: 20 additions & 0 deletions packages/@ember/-internals/metal/tests/tracked/validation_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'](
assert
) {
assert.expect(0);
class EmberObject {
get foo() {
notifyPropertyChange(this, 'foo');
return 123;
}
}
let obj = new EmberObject();
track(() => {
get(obj, 'foo');
});
}
}
);

0 comments on commit 34facf1

Please sign in to comment.