From d1b46a7149e763f736c1c210582f77ea3d5b2413 Mon Sep 17 00:00:00 2001 From: simonihmig Date: Sat, 10 Aug 2019 00:28:37 +0200 Subject: [PATCH] [BUGFIX beta] Ensures that observers are flushed after CPs are updated 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. --- .../@ember/-internals/metal/lib/computed.ts | 23 ++++++++------ .../-internals/metal/tests/observer_test.js | 31 +++++++++++++++++++ 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/computed.ts b/packages/@ember/-internals/metal/lib/computed.ts index 0c69540d9fb..83dc6078b13 100644 --- a/packages/@ember/-internals/metal/lib/computed.ts +++ b/packages/@ember/-internals/metal/lib/computed.ts @@ -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'; @@ -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); } diff --git a/packages/@ember/-internals/metal/tests/observer_test.js b/packages/@ember/-internals/metal/tests/observer_test.js index d83af0d45a5..b66a576577c 100644 --- a/packages/@ember/-internals/metal/tests/observer_test.js +++ b/packages/@ember/-internals/metal/tests/observer_test.js @@ -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 ) {