Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove private enumerable observers #16116

Merged
merged 1 commit into from
Jan 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 2 additions & 27 deletions packages/ember-runtime/lib/mixins/array.js
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
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
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
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
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
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