From d342a2a1dc89200a941040e7eefaaede161a39bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Mu=C3=B1oz?= Date: Thu, 11 Jan 2018 23:21:23 -0500 Subject: [PATCH] Remove private enumerable observers --- packages/ember-runtime/lib/mixins/array.js | 29 +------ .../ember-runtime/lib/mixins/enumerable.js | 80 +----------------- .../ember-runtime/tests/mixins/array_test.js | 69 +--------------- .../tests/mixins/enumerable_test.js | 81 +------------------ .../ember-runtime/tests/suites/enumerable.js | 23 ------ .../tests/suites/mutable_array/replace.js | 12 --- .../suites/mutable_enumerable/addObject.js | 11 --- .../suites/mutable_enumerable/removeObject.js | 12 --- .../mutable_enumerable/removeObjects.js | 12 --- 9 files changed, 6 insertions(+), 323 deletions(-) diff --git a/packages/ember-runtime/lib/mixins/array.js b/packages/ember-runtime/lib/mixins/array.js index 5449e2f99e1..fd524652143 100644 --- a/packages/ember-runtime/lib/mixins/array.js +++ b/packages/ember-runtime/lib/mixins/array.js @@ -59,8 +59,6 @@ export function objectAt(content, idx) { } export function arrayContentWillChange(array, startIdx, removeAmt, addAmt) { - let removing, lim; - // if no args are passed assume everything changes if (startIdx === undefined) { startIdx = 0; @@ -81,18 +79,7 @@ export function arrayContentWillChange(array, startIdx, removeAmt, addAmt) { sendEvent(array, '@array:before', [array, startIdx, removeAmt, addAmt]); - if (startIdx >= 0 && removeAmt >= 0 && get(array, 'hasEnumerableObservers')) { - removing = []; - lim = startIdx + removeAmt; - - for (let idx = startIdx; idx < lim; idx++) { - removing.push(objectAt(array, idx)); - } - } else { - removing = removeAmt; - } - - array.enumerableContentWillChange(removing, addAmt); + array.enumerableContentWillChange(removeAmt, addAmt); return array; } @@ -112,19 +99,7 @@ export function arrayContentDidChange(array, startIdx, removeAmt, addAmt) { } } - let adding; - if (startIdx >= 0 && addAmt >= 0 && get(array, 'hasEnumerableObservers')) { - adding = []; - let lim = startIdx + addAmt; - - for (let idx = startIdx; idx < lim; idx++) { - adding.push(objectAt(array, idx)); - } - } else { - adding = addAmt; - } - - array.enumerableContentDidChange(removeAmt, adding); + array.enumerableContentDidChange(removeAmt, addAmt); if (array.__each) { array.__each.arrayDidChange(array, startIdx, removeAmt, addAmt); diff --git a/packages/ember-runtime/lib/mixins/enumerable.js b/packages/ember-runtime/lib/mixins/enumerable.js index a17b7b2869c..4b97b67b955 100644 --- a/packages/ember-runtime/lib/mixins/enumerable.js +++ b/packages/ember-runtime/lib/mixins/enumerable.js @@ -14,11 +14,7 @@ import { aliasMethod, computed, propertyWillChange, - propertyDidChange, - addListener, - removeListener, - sendEvent, - hasListeners + propertyDidChange } from 'ember-metal'; import { assert } from 'ember-debug'; import compare from '../compare'; @@ -842,76 +838,6 @@ const Enumerable = Mixin.create({ // ENUMERABLE OBSERVERS // - /** - Registers an enumerable observer. Must implement `Ember.EnumerableObserver` - mixin. - - @method addEnumerableObserver - @param {Object} target - @param {Object} [opts] - @return this - @private - */ - addEnumerableObserver(target, opts) { - let willChange = (opts && opts.willChange) || 'enumerableWillChange'; - let didChange = (opts && opts.didChange) || 'enumerableDidChange'; - let hasObservers = get(this, 'hasEnumerableObservers'); - - if (!hasObservers) { - propertyWillChange(this, 'hasEnumerableObservers'); - } - - addListener(this, '@enumerable:before', target, willChange); - addListener(this, '@enumerable:change', target, didChange); - - if (!hasObservers) { - propertyDidChange(this, 'hasEnumerableObservers'); - } - - return this; - }, - - /** - Removes a registered enumerable observer. - - @method removeEnumerableObserver - @param {Object} target - @param {Object} [opts] - @return this - @private - */ - removeEnumerableObserver(target, opts) { - let willChange = (opts && opts.willChange) || 'enumerableWillChange'; - let didChange = (opts && opts.didChange) || 'enumerableDidChange'; - let hasObservers = get(this, 'hasEnumerableObservers'); - - if (hasObservers) { - propertyWillChange(this, 'hasEnumerableObservers'); - } - - removeListener(this, '@enumerable:before', target, willChange); - removeListener(this, '@enumerable:change', target, didChange); - - if (hasObservers) { - propertyDidChange(this, 'hasEnumerableObservers'); - } - - return this; - }, - - /** - Becomes true whenever the array currently has observers watching changes - on the array. - - @property hasEnumerableObservers - @type Boolean - @private - */ - hasEnumerableObservers: computed(function() { - return hasListeners(this, '@enumerable:change') || hasListeners(this, '@enumerable:before'); - }), - - /** Invoke this method just before the contents of your enumerable will change. You can either omit the parameters completely or pass the objects @@ -960,8 +886,6 @@ const Enumerable = Mixin.create({ propertyWillChange(this, 'length'); } - sendEvent(this, '@enumerable:before', [this, removing, adding]); - return this; }, @@ -1009,8 +933,6 @@ const Enumerable = Mixin.create({ adding = null; } - sendEvent(this, '@enumerable:change', [this, removing, adding]); - if (hasDelta) { propertyDidChange(this, 'length'); } diff --git a/packages/ember-runtime/tests/mixins/array_test.js b/packages/ember-runtime/tests/mixins/array_test.js index 41251041da5..88a9215b1f4 100644 --- a/packages/ember-runtime/tests/mixins/array_test.js +++ b/packages/ember-runtime/tests/mixins/array_test.js @@ -213,7 +213,7 @@ QUnit.module('notify array observers', { } }); -QUnit.test('should notify enumerable observers when called with no params', function(assert) { +QUnit.test('should notify array observers when called with no params', function(assert) { arrayContentWillChange(obj); assert.deepEqual(observer._before, [obj, 0, -1, -1]); @@ -238,7 +238,7 @@ QUnit.test('should notify when called with diff length items', function(assert) assert.deepEqual(observer._after, [obj, 0, 2, 1]); }); -QUnit.test('removing enumerable observer should disable', function(assert) { +QUnit.test('removing array observer should disable', function(assert) { removeArrayObserver(obj, observer); arrayContentWillChange(obj); assert.deepEqual(observer._before, null); @@ -247,71 +247,6 @@ QUnit.test('removing enumerable observer should disable', function(assert) { assert.deepEqual(observer._after, null); }); -// .......................................................... -// NOTIFY ENUMERABLE OBSERVER -// - -QUnit.module('notify enumerable observers as well', { - beforeEach(assert) { - obj = DummyArray.create(); - - observer = EmberObject.extend({ - enumerableWillChange() { - assert.equal(this._before, null); // should only call once - this._before = Array.prototype.slice.call(arguments); - }, - - enumerableDidChange() { - assert.equal(this._after, null); // should only call once - this._after = Array.prototype.slice.call(arguments); - } - }).create({ - _before: null, - _after: null - }); - - obj.addEnumerableObserver(observer); - }, - - afterEach() { - obj = observer = null; - } -}); - -QUnit.test('should notify enumerable observers when called with no params', function(assert) { - arrayContentWillChange(obj); - assert.deepEqual(observer._before, [obj, null, null], 'before'); - - arrayContentDidChange(obj); - assert.deepEqual(observer._after, [obj, null, null], 'after'); -}); - -// API variation that included items only -QUnit.test('should notify when called with same length items', function(assert) { - arrayContentWillChange(obj, 0, 1, 1); - assert.deepEqual(observer._before, [obj, ['ITEM-0'], 1], 'before'); - - arrayContentDidChange(obj, 0, 1, 1); - assert.deepEqual(observer._after, [obj, 1, ['ITEM-0']], 'after'); -}); - -QUnit.test('should notify when called with diff length items', function(assert) { - arrayContentWillChange(obj, 0, 2, 1); - assert.deepEqual(observer._before, [obj, ['ITEM-0', 'ITEM-1'], 1], 'before'); - - arrayContentDidChange(obj, 0, 2, 1); - assert.deepEqual(observer._after, [obj, 2, ['ITEM-0']], 'after'); -}); - -QUnit.test('removing enumerable observer should disable', function(assert) { - obj.removeEnumerableObserver(observer); - arrayContentWillChange(obj); - assert.deepEqual(observer._before, null, 'before'); - - arrayContentDidChange(obj); - assert.deepEqual(observer._after, null, 'after'); -}); - // .......................................................... // @each // diff --git a/packages/ember-runtime/tests/mixins/enumerable_test.js b/packages/ember-runtime/tests/mixins/enumerable_test.js index 7703f5d688c..355b291e984 100644 --- a/packages/ember-runtime/tests/mixins/enumerable_test.js +++ b/packages/ember-runtime/tests/mixins/enumerable_test.js @@ -184,7 +184,7 @@ let DummyEnum = EmberObject.extend(Enumerable, { length: 0 }); -let obj, observer; +let obj; // .......................................................... // NOTIFY ENUMERABLE PROPERTY @@ -278,82 +278,3 @@ QUnit.test('should notify when passed old index API with delta', function(assert obj.enumerableContentDidChange(1, 2); assert.equal(obj._after, 1); }); - -// .......................................................... -// NOTIFY ENUMERABLE OBSERVER -// - -QUnit.module('notify enumerable observers', { - beforeEach(assert) { - obj = DummyEnum.create(); - - observer = EmberObject.extend({ - enumerableWillChange() { - assert.equal(this._before, null); // should only call once - this._before = Array.prototype.slice.call(arguments); - }, - - enumerableDidChange() { - assert.equal(this._after, null); // should only call once - this._after = Array.prototype.slice.call(arguments); - } - }).create({ - _before: null, - _after: null - }); - - obj.addEnumerableObserver(observer); - }, - - afterEach() { - obj = observer = null; - } -}); - -QUnit.test('should notify enumerable observers when called with no params', function(assert) { - obj.enumerableContentWillChange(); - assert.deepEqual(observer._before, [obj, null, null]); - - obj.enumerableContentDidChange(); - assert.deepEqual(observer._after, [obj, null, null]); -}); - -// API variation that included items only -QUnit.test('should notify when called with same length items', function(assert) { - let added = ['foo']; - let removed = ['bar']; - - obj.enumerableContentWillChange(removed, added); - assert.deepEqual(observer._before, [obj, removed, added]); - - obj.enumerableContentDidChange(removed, added); - assert.deepEqual(observer._after, [obj, removed, added]); -}); - -QUnit.test('should notify when called with diff length items', function(assert) { - let added = ['foo', 'baz']; - let removed = ['bar']; - - obj.enumerableContentWillChange(removed, added); - assert.deepEqual(observer._before, [obj, removed, added]); - - obj.enumerableContentDidChange(removed, added); - assert.deepEqual(observer._after, [obj, removed, added]); -}); - -QUnit.test('should not notify when passed with indexes only', function(assert) { - obj.enumerableContentWillChange(1, 2); - assert.deepEqual(observer._before, [obj, 1, 2]); - - obj.enumerableContentDidChange(1, 2); - assert.deepEqual(observer._after, [obj, 1, 2]); -}); - -QUnit.test('removing enumerable observer should disable', function(assert) { - obj.removeEnumerableObserver(observer); - obj.enumerableContentWillChange(); - assert.deepEqual(observer._before, null); - - obj.enumerableContentDidChange(); - assert.deepEqual(observer._after, null); -}); diff --git a/packages/ember-runtime/tests/suites/enumerable.js b/packages/ember-runtime/tests/suites/enumerable.js index 6d376b9ab7a..a224b3b6386 100644 --- a/packages/ember-runtime/tests/suites/enumerable.js +++ b/packages/ember-runtime/tests/suites/enumerable.js @@ -130,29 +130,6 @@ const ObserverClass = EmberObject.extend({ */ timesCalled(key) { return this._keys[key] || 0; - }, - - /* - begins acting as an enumerable observer. - */ - observeEnumerable(obj) { - obj.addEnumerableObserver(this); - return this; - }, - - stopObserveEnumerable(obj) { - obj.removeEnumerableObserver(this); - return this; - }, - - enumerableWillChange() { - QUnit.config.current.assert.equal(this._before, null, 'should only call once'); - this._before = Array.prototype.slice.call(arguments); - }, - - enumerableDidChange() { - QUnit.config.current.assert.equal(this._after, null, 'should only call once'); - this._after = Array.prototype.slice.call(arguments); } }); diff --git a/packages/ember-runtime/tests/suites/mutable_array/replace.js b/packages/ember-runtime/tests/suites/mutable_array/replace.js index a67a131b729..4939abe74bc 100644 --- a/packages/ember-runtime/tests/suites/mutable_array/replace.js +++ b/packages/ember-runtime/tests/suites/mutable_array/replace.js @@ -147,18 +147,6 @@ suite.test('[A,B,C,D].replace(-1,1) => [A,B,C] + notify', function(assert) { assert.equal(observer.validate('firstObject'), false, 'should NOT have notified firstObject once'); }); -suite.test('Adding object should notify enumerable observer', function(assert) { - let fixtures = this.newFixture(4); - let obj = this.newObject(fixtures); - let observer = this.newObserver(obj).observeEnumerable(obj); - let item = this.newFixture(1)[0]; - - obj.replace(2, 2, [item]); - - assert.deepEqual(observer._before, [obj, [fixtures[2], fixtures[3]], 1], 'before'); - assert.deepEqual(observer._after, [obj, 2, [item]], 'after'); -}); - suite.test('Adding object should notify array observer', function(assert) { let fixtures = this.newFixture(4); let obj = this.newObject(fixtures); diff --git a/packages/ember-runtime/tests/suites/mutable_enumerable/addObject.js b/packages/ember-runtime/tests/suites/mutable_enumerable/addObject.js index 94fc38c3c08..1cd5ba44579 100644 --- a/packages/ember-runtime/tests/suites/mutable_enumerable/addObject.js +++ b/packages/ember-runtime/tests/suites/mutable_enumerable/addObject.js @@ -56,15 +56,4 @@ suite.test('[A,B,C].addObject(A) => [A,B,C] + NO notify', function(assert) { } }); -suite.test('Adding object should notify enumerable observer', function(assert) { - let obj = this.newObject(this.newFixture(3)); - let observer = this.newObserver(obj).observeEnumerable(obj); - let item = this.newFixture(1)[0]; - - obj.addObject(item); - - assert.deepEqual(observer._before, [obj, null, [item]]); - assert.deepEqual(observer._after, [obj, null, [item]]); -}); - export default suite; diff --git a/packages/ember-runtime/tests/suites/mutable_enumerable/removeObject.js b/packages/ember-runtime/tests/suites/mutable_enumerable/removeObject.js index 73047823e46..5cd444cb39e 100644 --- a/packages/ember-runtime/tests/suites/mutable_enumerable/removeObject.js +++ b/packages/ember-runtime/tests/suites/mutable_enumerable/removeObject.js @@ -58,16 +58,4 @@ suite.test('[A,B,C].removeObject(D) => [A,B,C]', function(assert) { } }); -suite.test('Removing object should notify enumerable observer', function(assert) { - let fixtures = this.newFixture(3); - let obj = this.newObject(fixtures); - let observer = this.newObserver(obj).observeEnumerable(obj); - let item = fixtures[1]; - - obj.removeObject(item); - - assert.deepEqual(observer._before, [obj, [item], null]); - assert.deepEqual(observer._after, [obj, [item], null]); -}); - export default suite; diff --git a/packages/ember-runtime/tests/suites/mutable_enumerable/removeObjects.js b/packages/ember-runtime/tests/suites/mutable_enumerable/removeObjects.js index f4b599cb592..5b638136fdb 100644 --- a/packages/ember-runtime/tests/suites/mutable_enumerable/removeObjects.js +++ b/packages/ember-runtime/tests/suites/mutable_enumerable/removeObjects.js @@ -168,16 +168,4 @@ suite.test('[A,B,C].removeObjects([D]) => [A,B,C]', function(assert) { } }); -suite.test('Removing objects should notify enumerable observer', function(assert) { - let fixtures = this.newFixture(3); - let obj = this.newObject(fixtures); - let observer = this.newObserver(obj).observeEnumerable(obj); - let item = fixtures[1]; - - obj.removeObjects([item]); - - assert.deepEqual(observer._before, [obj, [item], null]); - assert.deepEqual(observer._after, [obj, [item], null]); -}); - export default suite;