Skip to content

Commit

Permalink
Merge pull request #16116 from emberjs/remove-enumerable-observers
Browse files Browse the repository at this point in the history 
Remove private enumerable observers
  • Loading branch information
@rwjblue
rwjblue authored Jan 12, 2018
2 parents ee85052 + d342a2a commit 47f2404
Show file tree
Hide file tree
Showing 9 changed files with 6 additions and 323 deletions.
29 changes: 2 additions & 27 deletions packages/ember-runtime/lib/mixins/array.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Expand All @@ -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);
Expand Down
80 changes: 1 addition & 79 deletions packages/ember-runtime/lib/mixins/enumerable.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -960,8 +886,6 @@ const Enumerable = Mixin.create({
propertyWillChange(this, 'length');
}

sendEvent(this, '@enumerable:before', [this, removing, adding]);

return this;
},

Expand Down Expand Up @@ -1009,8 +933,6 @@ const Enumerable = Mixin.create({
adding = null;
}

sendEvent(this, '@enumerable:change', [this, removing, adding]);

if (hasDelta) {
propertyDidChange(this, 'length');
}
Expand Down
69 changes: 2 additions & 67 deletions packages/ember-runtime/tests/mixins/array_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]);

Expand All @@ -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);
Expand All @@ -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
//
Expand Down
81 changes: 1 addition & 80 deletions packages/ember-runtime/tests/mixins/enumerable_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ let DummyEnum = EmberObject.extend(Enumerable, {
length: 0
});

let obj, observer;
let obj;

// ..........................................................
// NOTIFY ENUMERABLE PROPERTY
Expand Down Expand Up @@ -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);
});
23 changes: 0 additions & 23 deletions packages/ember-runtime/tests/suites/enumerable.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});

Expand Down
12 changes: 0 additions & 12 deletions packages/ember-runtime/tests/suites/mutable_array/replace.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading

0 comments on commit 47f2404

Please sign in to comment.