Skip to content

Commit

Permalink
[BUGFIX beta] Ensures that observers are flushed after CPs are updated
Browse files Browse the repository at this point in the history
Ensures that observers are only flushed after CPs have fully updated.
Currently, they will fire before the `lastRevision` cache has been
updated, so they won't fire with the correct values or at the correct
times.
  • Loading branch information
simonihmig authored and Chris Garrett committed Aug 11, 2019
1 parent fb60c41 commit d9fe472
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 9 deletions.
23 changes: 14 additions & 9 deletions packages/@ember/-internals/metal/lib/computed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
import expandProperties from './expand_properties';
import { setObserverSuspended } from './observer';
import { defineProperty } from './properties';
import { notifyPropertyChange } from './property_events';
import { beginPropertyChanges, endPropertyChanges, notifyPropertyChange } from './property_events';
import { set } from './property_set';
import { tagForProperty, update } from './tags';
import { consume, track } from './tracked';
Expand Down Expand Up @@ -607,19 +607,24 @@ export class ComputedProperty extends ComputedDescriptor {
}

if (EMBER_METAL_TRACKED_PROPERTIES) {
let ret = this._set(obj, keyName, value);
let ret;

finishLazyChains(obj, keyName, ret);
try {
beginPropertyChanges();
ret = this._set(obj, keyName, value);

let propertyTag = tagForProperty(obj, keyName);
finishLazyChains(obj, keyName, ret);

if (this._dependentKeys !== undefined) {
update(propertyTag, getChainTagsForKeys(obj, this._dependentKeys));
}
let propertyTag = tagForProperty(obj, keyName);

setLastRevisionFor(obj, keyName, propertyTag.value());
if (this._dependentKeys !== undefined) {
update(propertyTag, getChainTagsForKeys(obj, this._dependentKeys));
}

return ret;
setLastRevisionFor(obj, keyName, propertyTag.value());
} finally {
endPropertyChanges();
}
} else {
return this.setWithSuspend(obj, keyName, value);
}
Expand Down
31 changes: 31 additions & 0 deletions packages/@ember/-internals/metal/tests/observer_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,37 @@ moduleFor(
assert.equal(count, 1, 'should have invoked observer');
}

// https://github.com/emberjs/ember.js/issues/18246
async ['@test observer should fire when computed property is modified'](assert) {
let obj = { bar: 'bar' };
defineProperty(
obj,
'foo',
computed('bar', {
get() {
return get(this, 'bar');
},
set(key, value) {
return value;
}
})
);

get(obj, 'foo');

let count = 0;
addObserver(obj, 'foo', function() {
assert.equal(get(obj, 'foo'), 'baz', 'should have invoked after prop change');
count++;
});

set(obj, 'foo', 'baz');
await runLoopSettled();

assert.equal(count, 1, 'should have invoked observer');
assert.equal(get(obj, 'foo'), 'baz', 'computed should have correct value');
}

async ['@test observer should continue to fire after dependent properties are accessed'](
assert
) {
Expand Down

0 comments on commit d9fe472

Please sign in to comment.