From a6e5a8208431583efb38b7d2818fb2991d58196e Mon Sep 17 00:00:00 2001 From: Kris Selden Date: Mon, 5 Dec 2016 17:23:43 -0800 Subject: [PATCH] [BUGFIX release] Fix issue #14672 PROPERTY_DID_CHANGE is the mechanism by which Ember's Glimmer refs are dirtied. Currently if something watches then unwatches an alias, the DK that is installed by the alias is removed. --- packages/ember-metal/lib/alias.js | 32 ++++++---- .../tests/property_did_change_hook.js | 64 +++++++++++++++++++ 2 files changed, 82 insertions(+), 14 deletions(-) create mode 100644 packages/ember-metal/tests/property_did_change_hook.js diff --git a/packages/ember-metal/lib/alias.js b/packages/ember-metal/lib/alias.js index 567d874afc8..a8a6d783046 100644 --- a/packages/ember-metal/lib/alias.js +++ b/packages/ember-metal/lib/alias.js @@ -14,6 +14,8 @@ import { removeDependentKeys } from './dependent_keys'; +const CONSUMED = {}; + export default function alias(altKey) { return new AliasedProperty(altKey); } @@ -34,28 +36,30 @@ AliasedProperty.prototype.setup = function(obj, keyName) { } }; -AliasedProperty.prototype._addDependentKeyIfMissing = function(obj, keyName) { +AliasedProperty.prototype.teardown = function(obj, keyName) { let meta = metaFor(obj); - if (!meta.peekDeps(this.altKey, keyName)) { - addDependentKeys(this, obj, keyName, meta); + if (meta.peekWatching(keyName)) { + removeDependentKeys(this, obj, keyName, meta); } }; -AliasedProperty.prototype._removeDependentKeyIfAdded = function(obj, keyName) { - let meta = metaFor(obj); - if (meta.peekDeps(this.altKey, keyName)) { - removeDependentKeys(this, obj, keyName, meta); - } +AliasedProperty.prototype.willWatch = function(obj, keyName) { + addDependentKeys(this, obj, keyName, metaFor(obj)); }; -AliasedProperty.prototype.willWatch = AliasedProperty.prototype._addDependentKeyIfMissing; -AliasedProperty.prototype.didUnwatch = AliasedProperty.prototype._removeDependentKeyIfAdded; -AliasedProperty.prototype.teardown = AliasedProperty.prototype._removeDependentKeyIfAdded; +AliasedProperty.prototype.didUnwatch = function(obj, keyName) { + removeDependentKeys(this, obj, keyName, metaFor(obj)); +}; AliasedProperty.prototype.get = function AliasedProperty_get(obj, keyName) { - this._addDependentKeyIfMissing(obj, keyName); - - return get(obj, this.altKey); + let ret = get(obj, this.altKey); + let meta = metaFor(obj); + let cache = meta.writableCache(); + if (cache[keyName] !== CONSUMED) { + cache[keyName] = CONSUMED; + addDependentKeys(this, obj, keyName, meta); + } + return ret; }; AliasedProperty.prototype.set = function AliasedProperty_set(obj, keyName, value) { diff --git a/packages/ember-metal/tests/property_did_change_hook.js b/packages/ember-metal/tests/property_did_change_hook.js new file mode 100644 index 00000000000..4a784f5aa18 --- /dev/null +++ b/packages/ember-metal/tests/property_did_change_hook.js @@ -0,0 +1,64 @@ +import { testBoth } from 'internal-test-helpers'; +import { PROPERTY_DID_CHANGE } from '../property_events'; +import { isWatching } from '../watching'; +import { defineProperty } from '../properties'; +import alias from '../alias'; +import { computed } from '../computed'; + +QUnit.module('PROPERTY_DID_CHANGE'); + +testBoth('alias and cp', function(get, set) { + let counts = {}; + let obj = { + child: {}, + [PROPERTY_DID_CHANGE](keyName) { + counts[keyName] = (counts[keyName] || 0) + 1; + } + }; + + defineProperty(obj, 'cost', alias('child.cost')); + defineProperty(obj, 'tax', alias('child.tax')); + + defineProperty(obj, 'total', computed('cost', 'tax', { + get() { + return get(this, 'cost') + get(this, 'tax'); + } + })); + + ok(!isWatching(obj, 'child.cost'), 'precond alias target `child.cost` is not watched'); + equal(get(obj, 'cost'), undefined); + // this is how PROPERTY_DID_CHANGE will get notified + ok(isWatching(obj, 'child.cost'), 'alias target `child.cost` is watched after consumption'); + + ok(!isWatching(obj, 'child.tax'), 'precond alias target `child.tax` is not watched'); + equal(get(obj, 'tax'), undefined); + // this is how PROPERTY_DID_CHANGE will get notified + ok(isWatching(obj, 'child.tax'), 'alias target `child.cost` is watched after consumption'); + + // increments the watching count on the alias itself to 1 + ok(isNaN(get(obj, 'total')), 'total is initialized'); + + // decrements the watching count on the alias itself to 0 + set(obj, 'child', { + cost: 399.00, + tax: 32.93 + }); + + // this should have called PROPERTY_DID_CHANGE for all of them + equal(counts['cost'], 1, 'PROPERTY_DID_CHANGE called with cost'); + equal(counts['tax'], 1, 'PROPERTY_DID_CHANGE called with tax'); + equal(counts['total'], 1, 'PROPERTY_DID_CHANGE called with total'); + + // we should still have a dependency installed + ok(isWatching(obj, 'child.cost'), 'watching child.cost'); + ok(isWatching(obj, 'child.tax'), 'watching child.tax'); + + set(obj, 'child', { + cost: 100.00, + tax: 10.00 + }); + + equal(counts['cost'], 2, 'PROPERTY_DID_CHANGE called with cost'); + equal(counts['tax'], 2, 'PROPERTY_DID_CHANGE called with tax'); + equal(counts['total'], 1, 'PROPERTY_DID_CHANGE called with total'); +});