diff --git a/packages/ember-metal/lib/mixin.js b/packages/ember-metal/lib/mixin.js index bee2969170f..e0ddddf1d18 100644 --- a/packages/ember-metal/lib/mixin.js +++ b/packages/ember-metal/lib/mixin.js @@ -30,6 +30,8 @@ import { Binding } from 'ember-metal/binding'; import { addObserver, removeObserver, + _addBeforeObserver, + _removeBeforeObserver, _suspendObserver } from 'ember-metal/observer'; import { @@ -352,11 +354,13 @@ function replaceObserversAndListeners(obj, key, observerOrListener) { var prev = obj[key]; if ('function' === typeof prev) { + updateObserversAndListeners(obj, key, prev, '__ember_observesBefore__', _removeBeforeObserver); updateObserversAndListeners(obj, key, prev, '__ember_observes__', removeObserver); updateObserversAndListeners(obj, key, prev, '__ember_listens__', removeListener); } if ('function' === typeof observerOrListener) { + updateObserversAndListeners(obj, key, observerOrListener, '__ember_observesBefore__', _addBeforeObserver); updateObserversAndListeners(obj, key, observerOrListener, '__ember_observes__', addObserver); updateObserversAndListeners(obj, key, observerOrListener, '__ember_listens__', addListener); } @@ -815,6 +819,75 @@ export function _immediateObserver() { return observer.apply(this, arguments); } +/** + When observers fire, they are called with the arguments `obj`, `keyName`. + + Note, `@each.property` observer is called per each add or replace of an element + and it's not called with a specific enumeration item. + + A `_beforeObserver` fires before a property changes. + + A `_beforeObserver` is an alternative form of `.observesBefore()`. + + ```javascript + App.PersonView = Ember.View.extend({ + friends: [{ name: 'Tom' }, { name: 'Stefan' }, { name: 'Kris' }], + + valueDidChange: Ember.observer('content.value', function(obj, keyName) { + // only run if updating a value already in the DOM + if (this.get('state') === 'inDOM') { + var color = obj.get(keyName) > this.changingFrom ? 'green' : 'red'; + // logic + } + }), + + friendsDidChange: Ember.observer('friends.@each.name', function(obj, keyName) { + // some logic + // obj.get(keyName) returns friends array + }) + }); + ``` + + Also available as `Function.prototype.observesBefore` if prototype extensions are + enabled. + + @method beforeObserver + @for Ember + @param {String} propertyNames* + @param {Function} func + @return func + @deprecated + @private +*/ +export function _beforeObserver(...args) { + var func = args.slice(-1)[0]; + var paths; + + var addWatchedProperty = function(path) { paths.push(path); }; + + var _paths = args.slice(0, -1); + + if (typeof func !== 'function') { + // revert to old, soft-deprecated argument ordering + + func = args[0]; + _paths = args.slice(1); + } + + paths = []; + + for (var i = 0; i < _paths.length; ++i) { + expandProperties(_paths[i], addWatchedProperty); + } + + if (typeof func !== 'function') { + throw new Ember.Error('Ember.beforeObserver called without a function'); + } + + func.__ember_observesBefore__ = paths; + return func; +} + export { IS_BINDING, Mixin, diff --git a/packages/ember-metal/lib/utils.js b/packages/ember-metal/lib/utils.js index 87bbbe0b1de..d4393586922 100644 --- a/packages/ember-metal/lib/utils.js +++ b/packages/ember-metal/lib/utils.js @@ -324,6 +324,7 @@ function _wrap(func, superFunc) { superWrapper.wrappedFunction = func; superWrapper.__ember_observes__ = func.__ember_observes__; + superWrapper.__ember_observesBefore__ = func.__ember_observesBefore__; superWrapper.__ember_listens__ = func.__ember_listens__; return superWrapper; diff --git a/packages/ember-metal/lib/watching.js b/packages/ember-metal/lib/watching.js index b8b2f00e33e..2d0cc45c691 100644 --- a/packages/ember-metal/lib/watching.js +++ b/packages/ember-metal/lib/watching.js @@ -53,6 +53,11 @@ export function isWatching(obj, key) { return (meta && meta.peekWatching(key)) > 0; } +export function watcherCount(obj, key) { + var meta = peekMeta(obj); + return (meta && meta.peekWatching(key)) || 0; +} + watch.flushPending = flushPendingChains; export function unwatch(obj, _keyPath, m) { diff --git a/packages/ember-metal/tests/observer_test.js b/packages/ember-metal/tests/observer_test.js index 0d845807034..ffc2074a34d 100644 --- a/packages/ember-metal/tests/observer_test.js +++ b/packages/ember-metal/tests/observer_test.js @@ -21,6 +21,7 @@ import { Mixin, mixin, observer, + _beforeObserver, _immediateObserver } from 'ember-metal/mixin'; import run from 'ember-metal/run_loop'; @@ -678,6 +679,53 @@ testBoth('observer should fire before dependent property is modified', function( equal(count, 1, 'should have invoked observer'); }); +testBoth('before observer watching multiple properties via brace expansion should fire when properties change', function (get, set) { + var obj = {}; + var count = 0; + + mixin(obj, { + fooAndBarWatcher: _beforeObserver('{foo,bar}', function () { + count++; + }) + }); + + set(obj, 'foo', 'foo'); + equal(count, 1, 'observer specified via brace expansion invoked on property change'); + + set(obj, 'bar', 'bar'); + equal(count, 2, 'observer specified via brace expansion invoked on property change'); + + set(obj, 'baz', 'baz'); + equal(count, 2, 'observer not invoked on unspecified property'); +}); + +testBoth('before observer watching multiple properties via brace expansion should fire when dependent property changes', function (get, set) { + var obj = { baz: 'Initial' }; + var count = 0; + + defineProperty(obj, 'foo', computed(function() { + return get(this, 'bar').toLowerCase(); + }).property('bar')); + + defineProperty(obj, 'bar', computed(function() { + return get(this, 'baz').toUpperCase(); + }).property('baz')); + + mixin(obj, { + fooAndBarWatcher: _beforeObserver('{foo,bar}', function () { + count++; + }) + }); + + get(obj, 'foo'); + set(obj, 'baz', 'Baz'); + // fire once for foo, once for bar + equal(count, 2, 'observer specified via brace expansion invoked on dependent property change'); + + set(obj, 'quux', 'Quux'); + equal(count, 2, 'observer not fired on unspecified property'); +}); + testBoth('_addBeforeObserver should propagate through prototype', function(get, set) { var obj = { foo: 'foo', count: 0 }; var obj2; diff --git a/packages/ember-runtime/lib/system/array_proxy.js b/packages/ember-runtime/lib/system/array_proxy.js index 931f73b804b..498bdeb9be5 100644 --- a/packages/ember-runtime/lib/system/array_proxy.js +++ b/packages/ember-runtime/lib/system/array_proxy.js @@ -4,7 +4,10 @@ import { isArray } from 'ember-runtime/utils'; import { computed } from 'ember-metal/computed'; -import { observer } from 'ember-metal/mixin'; +import { + _beforeObserver, + observer +} from 'ember-metal/mixin'; import { beginPropertyChanges, endPropertyChanges @@ -74,23 +77,7 @@ var ArrayProxy = EmberObject.extend(MutableArray, { @type Ember.Array @private */ - content: computed({ - get() { - return this._content; - }, - set(k, v) { - if (this._didInitArrayProxy) { - var oldContent = this._content; - var len = oldContent ? get(oldContent, 'length') : 0; - this.arrangedContentArrayWillChange(this, 0, len, undefined); - this.arrangedContentWillChange(this); - } - this._content = v; - return v; - } - }), - - + content: null, /** The array that the proxy pretends to be. In the default `ArrayProxy` @@ -99,7 +86,7 @@ var ArrayProxy = EmberObject.extend(MutableArray, { @property arrangedContent @private - */ + */ arrangedContent: alias('content'), /** @@ -137,7 +124,20 @@ var ArrayProxy = EmberObject.extend(MutableArray, { get(this, 'content').replace(idx, amt, objects); }, - _teardownContent(content) { + /** + Invoked when the content property is about to change. Notifies observers that the + entire array content will change. + + @private + @method _contentWillChange + */ + _contentWillChange: _beforeObserver('content', function() { + this._teardownContent(); + }), + + _teardownContent() { + var content = get(this, 'content'); + if (content) { content.removeArrayObserver(this, { willChange: 'contentArrayWillChange', @@ -180,7 +180,6 @@ var ArrayProxy = EmberObject.extend(MutableArray, { */ _contentDidChange: observer('content', function() { var content = get(this, 'content'); - this._teardownContent(this._prevContent); assert('Can\'t set ArrayProxy\'s content to itself', content !== this); @@ -189,7 +188,6 @@ var ArrayProxy = EmberObject.extend(MutableArray, { _setupContent() { var content = get(this, 'content'); - this._prevContent = content; if (content) { assert(`ArrayProxy expects an Array or Ember.ArrayProxy, but you passed ${typeof content}`, isArray(content) || content.isDestroyed); @@ -201,8 +199,17 @@ var ArrayProxy = EmberObject.extend(MutableArray, { } }, + _arrangedContentWillChange: _beforeObserver('arrangedContent', function() { + var arrangedContent = get(this, 'arrangedContent'); + var len = arrangedContent ? get(arrangedContent, 'length') : 0; + + this.arrangedContentArrayWillChange(this, 0, len, undefined); + this.arrangedContentWillChange(this); + + this._teardownArrangedContent(arrangedContent); + }), + _arrangedContentDidChange: observer('arrangedContent', function() { - this._teardownArrangedContent(this._prevArrangedContent); var arrangedContent = get(this, 'arrangedContent'); var len = arrangedContent ? get(arrangedContent, 'length') : 0; @@ -216,7 +223,6 @@ var ArrayProxy = EmberObject.extend(MutableArray, { _setupArrangedContent() { var arrangedContent = get(this, 'arrangedContent'); - this._prevArrangedContent = arrangedContent; if (arrangedContent) { assert(`ArrayProxy expects an Array or Ember.ArrayProxy, but you passed ${typeof arrangedContent}`, @@ -369,7 +375,6 @@ var ArrayProxy = EmberObject.extend(MutableArray, { }, init() { - this._didInitArrayProxy = true; this._super(...arguments); this._setupContent(); this._setupArrangedContent(); @@ -377,7 +382,7 @@ var ArrayProxy = EmberObject.extend(MutableArray, { willDestroy() { this._teardownArrangedContent(); - this._teardownContent(this.get('content')); + this._teardownContent(); } }); diff --git a/packages/ember-runtime/tests/system/array_proxy/watching_and_listening_test.js b/packages/ember-runtime/tests/system/array_proxy/watching_and_listening_test.js new file mode 100644 index 00000000000..c476e3eb1f3 --- /dev/null +++ b/packages/ember-runtime/tests/system/array_proxy/watching_and_listening_test.js @@ -0,0 +1,155 @@ +import { get } from 'ember-metal/property_get'; +import { listenersFor } from 'ember-metal/events'; +import { addObserver } from 'ember-metal/observer'; +import { defineProperty } from 'ember-metal/properties'; +import { watcherCount } from 'ember-metal/watching'; +import computed from 'ember-metal/computed'; +import ArrayProxy from 'ember-runtime/system/array_proxy'; +import { A } from 'ember-runtime/system/native_array'; + +function sortedListenersFor(obj, eventName) { + return listenersFor(obj, eventName).sort((listener1, listener2) => { + return (listener1[1] > listener2[1]) ? -1 : 1; + }); +} + +QUnit.module('ArrayProxy - watching and listening'); + +QUnit.test(`setting 'content' adds listeners correctly`, function() { + let content = A(); + let proxy = ArrayProxy.create(); + + deepEqual(sortedListenersFor(content, '@array:before'), []); + deepEqual(sortedListenersFor(content, '@array:change'), []); + + proxy.set('content', content); + + deepEqual( + sortedListenersFor(content, '@array:before'), + [[proxy, 'contentArrayWillChange'], [proxy, 'arrangedContentArrayWillChange']] + ); + deepEqual( + sortedListenersFor(content, '@array:change'), + [[proxy, 'contentArrayDidChange'], [proxy, 'arrangedContentArrayDidChange']] + ); +}); + +QUnit.test(`changing 'content' adds and removes listeners correctly`, function() { + let content1 = A(); + let content2 = A(); + let proxy = ArrayProxy.create({ content: content1 }); + + deepEqual( + sortedListenersFor(content1, '@array:before'), + [[proxy, 'contentArrayWillChange'], [proxy, 'arrangedContentArrayWillChange']] + ); + deepEqual( + sortedListenersFor(content1, '@array:change'), + [[proxy, 'contentArrayDidChange'], [proxy, 'arrangedContentArrayDidChange']] + ); + + proxy.set('content', content2); + + deepEqual(sortedListenersFor(content1, '@array:before'), []); + deepEqual(sortedListenersFor(content1, '@array:change'), []); + deepEqual( + sortedListenersFor(content2, '@array:before'), + [[proxy, 'contentArrayWillChange'], [proxy, 'arrangedContentArrayWillChange']] + ); + deepEqual( + sortedListenersFor(content2, '@array:change'), + [[proxy, 'contentArrayDidChange'], [proxy, 'arrangedContentArrayDidChange']] + ); +}); + +QUnit.test(`regression test for https://github.com/emberjs/ember.js/issues/12475`, function() { + let item1a = { id: 1 }; + let item1b = { id: 2 }; + let item1c = { id: 3 }; + let content1 = A([item1a, item1b, item1c]); + + let proxy = ArrayProxy.create({ content: content1 }); + let obj = { proxy }; + + defineProperty(obj, 'ids', computed('proxy.@each.id', function() { + return get(this, 'proxy').mapBy('id'); + })); + + // These manually added observers are to simulate the observers added by the + // rendering process in a template like: + // + // {{#each items as |item|}} + // {{item.id}} + // {{/each}} + addObserver(item1a, 'id', function() { }); + addObserver(item1b, 'id', function() { }); + addObserver(item1c, 'id', function() { }); + + // The EachProxy has not yet been consumed. Only the manually added + // observers are watching. + equal(watcherCount(item1a, 'id'), 1); + equal(watcherCount(item1b, 'id'), 1); + equal(watcherCount(item1c, 'id'), 1); + + // Consume the each proxy. This causes the EachProxy to add two observers + // per item: one for "before" events and one for "after" events. + deepEqual(get(obj, 'ids'), [1, 2, 3]); + + // For each item, the two each proxy observers and one manual added observer + // are watching. + equal(watcherCount(item1a, 'id'), 3); + equal(watcherCount(item1b, 'id'), 3); + equal(watcherCount(item1c, 'id'), 3); + + // This should be a no-op because observers do not fire if the value + // 1. is an object and 2. is the same as the old value. + proxy.set('content', content1); + + equal(watcherCount(item1a, 'id'), 3); + equal(watcherCount(item1b, 'id'), 3); + equal(watcherCount(item1c, 'id'), 3); + + // This is repeated to catch the regression. It should still be a no-op. + proxy.set('content', content1); + + equal(watcherCount(item1a, 'id'), 3); + equal(watcherCount(item1b, 'id'), 3); + equal(watcherCount(item1c, 'id'), 3); + + // Set the content to a new array with completely different items and + // repeat the process. + let item2a = { id: 4 }; + let item2b = { id: 5 }; + let item2c = { id: 6 }; + let content2 = A([item2a, item2b, item2c]); + + addObserver(item2a, 'id', function() { }); + addObserver(item2b, 'id', function() { }); + addObserver(item2c, 'id', function() { }); + + proxy.set('content', content2); + + deepEqual(get(obj, 'ids'), [4, 5, 6]); + + equal(watcherCount(item2a, 'id'), 3); + equal(watcherCount(item2b, 'id'), 3); + equal(watcherCount(item2c, 'id'), 3); + + // Ensure that the observers added by the EachProxy on all items in the + // first content array have been torn down. + equal(watcherCount(item1a, 'id'), 1); + equal(watcherCount(item1b, 'id'), 1); + equal(watcherCount(item1c, 'id'), 1); + + proxy.set('content', content2); + + equal(watcherCount(item2a, 'id'), 3); + equal(watcherCount(item2b, 'id'), 3); + equal(watcherCount(item2c, 'id'), 3); + + proxy.set('content', content2); + + equal(watcherCount(item2a, 'id'), 3); + equal(watcherCount(item2b, 'id'), 3); + equal(watcherCount(item2c, 'id'), 3); +});