From dabdd894423ecb148665b0f7fea0b72a1bde521d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Mu=C3=B1oz?= Date: Wed, 24 Jan 2018 21:00:17 -0500 Subject: [PATCH] [CLEANUP] Migrate to notifyPropertyChange internally and deprecate propertyDidChange --- .../tests/integration/syntax/each-test.js | 6 ++-- packages/ember-metal/lib/computed.js | 4 +-- packages/ember-metal/lib/index.js | 1 + packages/ember-metal/lib/property_events.js | 28 ++++++++++++++++--- packages/ember-metal/lib/property_set.js | 4 +-- packages/ember-metal/tests/chains_test.js | 4 +-- packages/ember-metal/tests/observer_test.js | 4 +-- .../ember-metal/tests/performance_test.js | 4 +-- packages/ember-runtime/lib/mixins/-proxy.js | 4 +-- packages/ember-runtime/lib/mixins/array.js | 16 +++++------ .../ember-runtime/lib/mixins/observable.js | 19 ++++++------- .../mixins/observable/observable_test.js | 2 +- .../mixins/observable/propertyChanges_test.js | 2 +- .../tests/mixins/observable_test.js | 8 ++++++ packages/ember/lib/index.js | 1 + packages/ember/tests/reexports_test.js | 2 +- 16 files changed, 68 insertions(+), 41 deletions(-) diff --git a/packages/ember-glimmer/tests/integration/syntax/each-test.js b/packages/ember-glimmer/tests/integration/syntax/each-test.js index 3b218f8b5f6..ccc28ddc215 100644 --- a/packages/ember-glimmer/tests/integration/syntax/each-test.js +++ b/packages/ember-glimmer/tests/integration/syntax/each-test.js @@ -1,4 +1,4 @@ -import { get, set, propertyDidChange } from 'ember-metal'; +import { get, set, notifyPropertyChange } from 'ember-metal'; import { applyMixins, strip } from '../../utils/abstract-test-case'; import { moduleFor, RenderingTest } from '../../utils/test-case'; import { A as emberA, ArrayProxy, RSVP } from 'ember-runtime'; @@ -83,8 +83,8 @@ class ArrayLike { } arrayContentDidChange() { - propertyDidChange(this, '[]'); - propertyDidChange(this, 'length'); + notifyPropertyChange(this, '[]'); + notifyPropertyChange(this, 'length'); } } diff --git a/packages/ember-metal/lib/computed.js b/packages/ember-metal/lib/computed.js index 551c9db53e7..a61e06a8af9 100644 --- a/packages/ember-metal/lib/computed.js +++ b/packages/ember-metal/lib/computed.js @@ -8,7 +8,7 @@ import { defineProperty } from './properties'; import { - propertyDidChange + notifyPropertyChange } from './property_events'; import { addDependentKeys, @@ -418,7 +418,7 @@ ComputedPropertyPrototype._set = function computedPropertySet(obj, keyName, valu cache[keyName] = ret === undefined ? UNDEFINED : ret; - propertyDidChange(obj, keyName, meta); + notifyPropertyChange(obj, keyName, meta); return ret; }; diff --git a/packages/ember-metal/lib/index.js b/packages/ember-metal/lib/index.js index a9651d911a3..79e0b624566 100644 --- a/packages/ember-metal/lib/index.js +++ b/packages/ember-metal/lib/index.js @@ -57,6 +57,7 @@ export { beginPropertyChanges, changeProperties, endPropertyChanges, + notifyPropertyChange, overrideChains, propertyDidChange, propertyWillChange, diff --git a/packages/ember-metal/lib/property_events.js b/packages/ember-metal/lib/property_events.js index 8de97c2eb76..6f3427b662d 100644 --- a/packages/ember-metal/lib/property_events.js +++ b/packages/ember-metal/lib/property_events.js @@ -48,6 +48,25 @@ function propertyWillChange() { ); } +/** + @method propertyDidChange + @for Ember + @private +*/ +function propertyDidChange(obj, keyName, _meta) { + deprecate( + `'propertyDidChange' is deprecated in favor of 'notifyPropertyChange'. It is safe to change this call to 'notifyPropertyChange'.`, + false, + { + id: 'ember-metal.deprecate-propertyDidChange', + until: '3.5.0', + url: 'https://emberjs.com/deprecations/v3.x/#toc_ember-metal-deprecate-propertyWillChange-and-propertyDidChange' + } + ); + + notifyPropertyChange(obj, keyName, _meta); +} + /** This function is called just after an object property has changed. It will notify any observers and clear caches among other things. @@ -56,7 +75,7 @@ function propertyWillChange() { reason you can't directly watch a property you can invoke this method manually. - @method propertyDidChange + @method notifyPropertyChange @for Ember @param {Object} obj The object with the property that will change @param {String} keyName The property key (or path) that will change. @@ -64,7 +83,7 @@ function propertyWillChange() { @return {void} @private */ -function propertyDidChange(obj, keyName, _meta) { +function notifyPropertyChange(obj, keyName, _meta) { let meta = _meta === undefined ? peekMeta(obj) : _meta; let hasMeta = meta !== undefined; @@ -109,7 +128,7 @@ function dependentKeysDidChange(obj, depKey, meta) { seen = DID_SEEN = {}; } - iterDeps(propertyDidChange, obj, depKey, seen, meta); + iterDeps(notifyPropertyChange, obj, depKey, seen, meta); if (top) { DID_SEEN = null; @@ -147,7 +166,7 @@ function iterDeps(method, obj, depKey, seen, meta) { function chainsDidChange(obj, keyName, meta) { let chainWatchers = meta.readableChainWatchers(); if (chainWatchers !== undefined) { - chainWatchers.notify(keyName, true, propertyDidChange); + chainWatchers.notify(keyName, true, notifyPropertyChange); } } @@ -251,6 +270,7 @@ function notifyObservers(obj, keyName, meta) { export { propertyWillChange, propertyDidChange, + notifyPropertyChange, overrideChains, beginPropertyChanges, endPropertyChanges, diff --git a/packages/ember-metal/lib/property_set.js b/packages/ember-metal/lib/property_set.js index 267debfb06a..cc9fef783ac 100644 --- a/packages/ember-metal/lib/property_set.js +++ b/packages/ember-metal/lib/property_set.js @@ -2,7 +2,7 @@ import { toString } from 'ember-utils'; import { assert, Error as EmberError } from 'ember-debug'; import { getPossibleMandatoryProxyValue, _getPath as getPath } from './property_get'; import { - propertyDidChange + notifyPropertyChange } from './property_events'; import { @@ -82,7 +82,7 @@ export function set(obj, keyName, value, tolerant) { obj[keyName] = value; } - propertyDidChange(obj, keyName, meta); + notifyPropertyChange(obj, keyName, meta); } return value; diff --git a/packages/ember-metal/tests/chains_test.js b/packages/ember-metal/tests/chains_test.js index d6e90a7c7a4..b82118209a7 100644 --- a/packages/ember-metal/tests/chains_test.js +++ b/packages/ember-metal/tests/chains_test.js @@ -5,7 +5,7 @@ import { finishChains, defineProperty, computed, - propertyDidChange, + notifyPropertyChange, peekMeta, meta } from '..'; @@ -65,7 +65,7 @@ QUnit.test('observer and CP chains', function(assert) { */ // invalidate qux - propertyDidChange(obj, 'qux'); + notifyPropertyChange(obj, 'qux'); // CP chain is blown away diff --git a/packages/ember-metal/tests/observer_test.js b/packages/ember-metal/tests/observer_test.js index 2940fa96cb9..815ae84b33c 100644 --- a/packages/ember-metal/tests/observer_test.js +++ b/packages/ember-metal/tests/observer_test.js @@ -3,7 +3,7 @@ import { testBoth } from 'internal-test-helpers'; import { addObserver, removeObserver, - propertyDidChange, + notifyPropertyChange, defineProperty, computed, cacheFor, @@ -76,7 +76,7 @@ testBoth('observer should continue to fire after dependent properties are access get(obj, 'anotherProp'); for (let i = 0; i < 10; i++) { - propertyDidChange(obj, 'prop'); + notifyPropertyChange(obj, 'prop'); } assert.equal(observerCount, 10, 'should continue to fire indefinitely'); diff --git a/packages/ember-metal/tests/performance_test.js b/packages/ember-metal/tests/performance_test.js index 71cb2c51957..9557bb5b536 100644 --- a/packages/ember-metal/tests/performance_test.js +++ b/packages/ember-metal/tests/performance_test.js @@ -3,7 +3,7 @@ import { get, computed, defineProperty, - propertyDidChange, + notifyPropertyChange, beginPropertyChanges, endPropertyChanges, addObserver @@ -59,7 +59,7 @@ moduleFor('Computed Properties - Number of times evaluated', class extends Abstr addObserver(foo, 'bar.baz.bam', function() {}); - propertyDidChange(get(foo, 'bar.baz'), 'bam'); + notifyPropertyChange(get(foo, 'bar.baz'), 'bam'); assert.equal(count, 0, 'should not have recomputed property'); } diff --git a/packages/ember-runtime/lib/mixins/-proxy.js b/packages/ember-runtime/lib/mixins/-proxy.js index 319019d1b58..0152e7129b3 100644 --- a/packages/ember-runtime/lib/mixins/-proxy.js +++ b/packages/ember-runtime/lib/mixins/-proxy.js @@ -9,7 +9,7 @@ import { meta, addObserver, removeObserver, - propertyDidChange, + notifyPropertyChange, defineProperty, Mixin, tagFor, @@ -22,7 +22,7 @@ import { bool } from '../computed/computed_macros'; function contentPropertyDidChange(content, contentKey) { let key = contentKey.slice(8); // remove "content." if (key in this) { return; } // if shadowed in proxy - propertyDidChange(this, key); + notifyPropertyChange(this, key); } class ProxyTag extends CachedTag { diff --git a/packages/ember-runtime/lib/mixins/array.js b/packages/ember-runtime/lib/mixins/array.js index 1519a163b84..df69ed93d71 100644 --- a/packages/ember-runtime/lib/mixins/array.js +++ b/packages/ember-runtime/lib/mixins/array.js @@ -10,7 +10,7 @@ import { isNone, aliasMethod, Mixin, - propertyDidChange, + notifyPropertyChange, addListener, removeListener, sendEvent, @@ -44,7 +44,7 @@ function arrayObserversHelper(obj, target, opts, operation, notify) { operation(obj, '@array:change', target, didChange); if (hasObservers === notify) { - propertyDidChange(obj, 'hasArrayObservers'); + notifyPropertyChange(obj, 'hasArrayObservers'); } return obj; @@ -102,10 +102,10 @@ export function arrayContentDidChange(array, startIdx, removeAmt, addAmt) { } if (addAmt < 0 || removeAmt < 0 || addAmt - removeAmt !== 0) { - propertyDidChange(array, 'length'); + notifyPropertyChange(array, 'length'); } - propertyDidChange(array, '[]'); + notifyPropertyChange(array, '[]'); if (array.__each) { array.__each.arrayDidChange(array, startIdx, removeAmt, addAmt); @@ -124,14 +124,14 @@ export function arrayContentDidChange(array, startIdx, removeAmt, addAmt) { let normalStartIdx = startIdx < 0 ? previousLength + startIdx : startIdx; if (cache.firstObject !== undefined && normalStartIdx === 0) { - propertyDidChange(array, 'firstObject', meta); + notifyPropertyChange(array, 'firstObject', meta); } if (cache.lastObject !== undefined) { let previousLastIndex = previousLength - 1; let lastAffectedIndex = normalStartIdx + removedAmount; if (previousLastIndex < lastAffectedIndex) { - propertyDidChange(array, 'lastObject', meta); + notifyPropertyChange(array, 'lastObject', meta); } } } @@ -1286,7 +1286,7 @@ EachProxy.prototype = { if (lim > 0) { addObserverForContentKey(content, key, this, idx, lim); } - propertyDidChange(this, key, meta); + notifyPropertyChange(this, key, meta); } }, @@ -1334,7 +1334,7 @@ EachProxy.prototype = { }, contentKeyDidChange(obj, keyName) { - propertyDidChange(this, keyName); + notifyPropertyChange(this, keyName); } }; diff --git a/packages/ember-runtime/lib/mixins/observable.js b/packages/ember-runtime/lib/mixins/observable.js index 3c0c0cb8379..f6c311f9c4b 100644 --- a/packages/ember-runtime/lib/mixins/observable.js +++ b/packages/ember-runtime/lib/mixins/observable.js @@ -13,6 +13,7 @@ import { beginPropertyChanges, propertyWillChange, propertyDidChange, + notifyPropertyChange, endPropertyChanges, addObserver, removeObserver, @@ -279,16 +280,7 @@ export default Mixin.create({ }, /** - Notify the observer system that a property has just changed. - - Sometimes you need to change a value directly or indirectly without - actually calling `get()` or `set()` on it. In this case, you can use this - method instead. Calling this method will notify all observers that the - property has potentially changed value. - @method propertyDidChange - @param {String} keyName The property key that has just changed. - @return {Observable} @private */ propertyDidChange(keyName) { @@ -297,7 +289,12 @@ export default Mixin.create({ }, /** - Convenience method to call `propertyDidChange`. + Notify the observer system that a property has just changed. + + Sometimes you need to change a value directly or indirectly without + actually calling `get()` or `set()` on it. In this case, you can use this + method instead. Calling this method will notify all observers that the + property has potentially changed value. @method notifyPropertyChange @param {String} keyName The property key to be notified about. @@ -305,7 +302,7 @@ export default Mixin.create({ @public */ notifyPropertyChange(keyName) { - this.propertyDidChange(keyName); + notifyPropertyChange(this, keyName); return this; }, diff --git a/packages/ember-runtime/tests/legacy_1x/mixins/observable/observable_test.js b/packages/ember-runtime/tests/legacy_1x/mixins/observable/observable_test.js index 3f3912b853a..00b90745c6b 100644 --- a/packages/ember-runtime/tests/legacy_1x/mixins/observable/observable_test.js +++ b/packages/ember-runtime/tests/legacy_1x/mixins/observable/observable_test.js @@ -414,7 +414,7 @@ moduleFor('Computed properties', class extends AbstractTestCase { object.get('computedCached'); // should run func object.get('computedCached'); // should not run func - object.propertyDidChange('computedCached'); + object.notifyPropertyChange('computedCached'); object.get('computedCached'); // should run again assert.equal(object.computedCachedCalls.length, 2, 'should have invoked method 2x'); diff --git a/packages/ember-runtime/tests/legacy_1x/mixins/observable/propertyChanges_test.js b/packages/ember-runtime/tests/legacy_1x/mixins/observable/propertyChanges_test.js index 663b5fa0ef5..8bb43c44c8a 100644 --- a/packages/ember-runtime/tests/legacy_1x/mixins/observable/propertyChanges_test.js +++ b/packages/ember-runtime/tests/legacy_1x/mixins/observable/propertyChanges_test.js @@ -91,7 +91,7 @@ moduleFor('object.propertyChanges', class extends AbstractTestCase { ObjectA.set('foo', 'changeFooValue'); // Indicate the subscribers of foo that the value has just changed - ObjectA.propertyDidChange('foo', null); + ObjectA.notifyPropertyChange('foo', null); // Values of prop has just changed assert.equal(ObjectA.prop, 'changedPropValue'); diff --git a/packages/ember-runtime/tests/mixins/observable_test.js b/packages/ember-runtime/tests/mixins/observable_test.js index fca32a8ccb1..92446c62339 100644 --- a/packages/ember-runtime/tests/mixins/observable_test.js +++ b/packages/ember-runtime/tests/mixins/observable_test.js @@ -104,3 +104,11 @@ QUnit.test('propertyWillChange triggers a deprecation warning', function () { obj.propertyWillChange('foo'); }, /'propertyWillChange' is deprecated and has no effect. It is safe to remove this call./); }); + +QUnit.test('propertyDidChange triggers a deprecation warning', function () { + let obj = EmberObject.create(); + + expectDeprecation(() => { + obj.propertyDidChange('foo'); + }, /'propertyDidChange' is deprecated in favor of 'notifyPropertyChange'. It is safe to change this call to 'notifyPropertyChange'./); +}); diff --git a/packages/ember/lib/index.js b/packages/ember/lib/index.js index f0a07055c55..82ab3abf4e8 100644 --- a/packages/ember/lib/index.js +++ b/packages/ember/lib/index.js @@ -90,6 +90,7 @@ Ember.run = metal.run; Ember._ObserverSet = metal.ObserverSet; Ember.propertyWillChange = metal.propertyWillChange; Ember.propertyDidChange = metal.propertyDidChange; +Ember.notifyPropertyChange = metal.notifyPropertyChange; Ember.overrideChains = metal.overrideChains; Ember.beginPropertyChanges = metal.beginPropertyChanges; Ember.endPropertyChanges = metal.endPropertyChanges; diff --git a/packages/ember/tests/reexports_test.js b/packages/ember/tests/reexports_test.js index ed871bcac11..fbf9c13d0de 100644 --- a/packages/ember/tests/reexports_test.js +++ b/packages/ember/tests/reexports_test.js @@ -75,9 +75,9 @@ let allExports =[ ['_ObserverSet', 'ember-metal', 'ObserverSet'], ['propertyWillChange', 'ember-metal'], ['propertyDidChange', 'ember-metal'], + ['notifyPropertyChange', 'ember-metal'], ['overrideChains', 'ember-metal'], ['beginPropertyChanges', 'ember-metal'], - ['beginPropertyChanges', 'ember-metal'], ['endPropertyChanges', 'ember-metal'], ['changeProperties', 'ember-metal'], ['defineProperty', 'ember-metal'],