Skip to content

Commit

Permalink
[CLEANUP] Migrate to notifyPropertyChange internally and deprecate pr…
Browse files Browse the repository at this point in the history
…opertyDidChange
  • Loading branch information
mmun committed Jan 25, 2018
1 parent 446053d commit dabdd89
Show file tree
Hide file tree
Showing 16 changed files with 68 additions and 41 deletions.
6 changes: 3 additions & 3 deletions packages/ember-glimmer/tests/integration/syntax/each-test.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -83,8 +83,8 @@ class ArrayLike {
}

arrayContentDidChange() {
propertyDidChange(this, '[]');
propertyDidChange(this, 'length');
notifyPropertyChange(this, '[]');
notifyPropertyChange(this, 'length');
}

}
Expand Down
4 changes: 2 additions & 2 deletions packages/ember-metal/lib/computed.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
defineProperty
} from './properties';
import {
propertyDidChange
notifyPropertyChange
} from './property_events';
import {
addDependentKeys,
Expand Down Expand Up @@ -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;
};
Expand Down
1 change: 1 addition & 0 deletions packages/ember-metal/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export {
beginPropertyChanges,
changeProperties,
endPropertyChanges,
notifyPropertyChange,
overrideChains,
propertyDidChange,
propertyWillChange,
Expand Down
28 changes: 24 additions & 4 deletions packages/ember-metal/lib/property_events.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -56,15 +75,15 @@ 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.
@param {Meta} meta The objects meta.
@return {void}
@private
*/
function propertyDidChange(obj, keyName, _meta) {
function notifyPropertyChange(obj, keyName, _meta) {
let meta = _meta === undefined ? peekMeta(obj) : _meta;
let hasMeta = meta !== undefined;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -251,6 +270,7 @@ function notifyObservers(obj, keyName, meta) {
export {
propertyWillChange,
propertyDidChange,
notifyPropertyChange,
overrideChains,
beginPropertyChanges,
endPropertyChanges,
Expand Down
4 changes: 2 additions & 2 deletions packages/ember-metal/lib/property_set.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -82,7 +82,7 @@ export function set(obj, keyName, value, tolerant) {
obj[keyName] = value;
}

propertyDidChange(obj, keyName, meta);
notifyPropertyChange(obj, keyName, meta);
}

return value;
Expand Down
4 changes: 2 additions & 2 deletions packages/ember-metal/tests/chains_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
finishChains,
defineProperty,
computed,
propertyDidChange,
notifyPropertyChange,
peekMeta,
meta
} from '..';
Expand Down Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions packages/ember-metal/tests/observer_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { testBoth } from 'internal-test-helpers';
import {
addObserver,
removeObserver,
propertyDidChange,
notifyPropertyChange,
defineProperty,
computed,
cacheFor,
Expand Down Expand Up @@ -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');
Expand Down
4 changes: 2 additions & 2 deletions packages/ember-metal/tests/performance_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
get,
computed,
defineProperty,
propertyDidChange,
notifyPropertyChange,
beginPropertyChanges,
endPropertyChanges,
addObserver
Expand Down Expand Up @@ -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');
}
Expand Down
4 changes: 2 additions & 2 deletions packages/ember-runtime/lib/mixins/-proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
meta,
addObserver,
removeObserver,
propertyDidChange,
notifyPropertyChange,
defineProperty,
Mixin,
tagFor,
Expand All @@ -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 {
Expand Down
16 changes: 8 additions & 8 deletions packages/ember-runtime/lib/mixins/array.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
isNone,
aliasMethod,
Mixin,
propertyDidChange,
notifyPropertyChange,
addListener,
removeListener,
sendEvent,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -1286,7 +1286,7 @@ EachProxy.prototype = {
if (lim > 0) {
addObserverForContentKey(content, key, this, idx, lim);
}
propertyDidChange(this, key, meta);
notifyPropertyChange(this, key, meta);
}
},

Expand Down Expand Up @@ -1334,7 +1334,7 @@ EachProxy.prototype = {
},

contentKeyDidChange(obj, keyName) {
propertyDidChange(this, keyName);
notifyPropertyChange(this, keyName);
}
};

Expand Down
19 changes: 8 additions & 11 deletions packages/ember-runtime/lib/mixins/observable.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
beginPropertyChanges,
propertyWillChange,
propertyDidChange,
notifyPropertyChange,
endPropertyChanges,
addObserver,
removeObserver,
Expand Down Expand Up @@ -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) {
Expand All @@ -297,15 +289,20 @@ 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.
@return {Observable}
@public
*/
notifyPropertyChange(keyName) {
this.propertyDidChange(keyName);
notifyPropertyChange(this, keyName);
return this;
},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
8 changes: 8 additions & 0 deletions packages/ember-runtime/tests/mixins/observable_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'./);
});
1 change: 1 addition & 0 deletions packages/ember/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/ember/tests/reexports_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down

0 comments on commit dabdd89

Please sign in to comment.