Skip to content

Commit

Permalink
Merge pull request #16101 from emberjs/array-proxy-cleanup
Browse files Browse the repository at this point in the history
Remove legacy ArrayProxy features
  • Loading branch information
rwjblue authored Jan 10, 2018
2 parents 6afb824 + 78f94cb commit 4d5ceed
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 167 deletions.
91 changes: 3 additions & 88 deletions packages/ember-runtime/lib/system/array_proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ import { assert, Error as EmberError } from 'ember-debug';
const OUT_OF_RANGE_EXCEPTION = 'Index out of range';
const EMPTY = [];

function K() { return this; }


/**
An ArrayProxy wraps any other object that implements `Ember.Array` and/or
Expand Down Expand Up @@ -76,7 +74,7 @@ export default EmberObject.extend(MutableArray, {
@property content
@type EmberArray
@private
@public
*/
content: null,

Expand All @@ -86,7 +84,7 @@ export default EmberObject.extend(MutableArray, {
can override this property to provide things like sorting and filtering.
@property arrangedContent
@private
@public
*/
arrangedContent: alias('content'),

Expand Down Expand Up @@ -125,87 +123,11 @@ export default EmberObject.extend(MutableArray, {
get(this, 'content').replace(idx, amt, objects);
},

/**
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() {
let content = get(this, 'content');

if (content) {
removeArrayObserver(content, this, {
willChange: 'contentArrayWillChange',
didChange: 'contentArrayDidChange'
});
}
},

/**
Override to implement content array `willChange` observer.
@method contentArrayWillChange
@param {EmberArray} contentArray the content array
@param {Number} start starting index of the change
@param {Number} removeCount count of items removed
@param {Number} addCount count of items added
@private
*/
contentArrayWillChange: K,
/**
Override to implement content array `didChange` observer.
@method contentArrayDidChange
@param {EmberArray} contentArray the content array
@param {Number} start starting index of the change
@param {Number} removeCount count of items removed
@param {Number} addCount count of items added
@private
*/
contentArrayDidChange: K,

/**
Invoked when the content property changes. Notifies observers that the
entire array content has changed.
@private
@method _contentDidChange
*/
_contentDidChange: observer('content', function() {
let content = get(this, 'content');

assert('Can\'t set ArrayProxy\'s content to itself', content !== this);

this._setupContent();
}),

_setupContent() {
let content = get(this, 'content');

if (content) {
assert(`ArrayProxy expects an Array or Ember.ArrayProxy, but you passed ${typeof content}`, isArray(content) || content.isDestroyed);

addArrayObserver(content, this, {
willChange: 'contentArrayWillChange',
didChange: 'contentArrayDidChange'
});
}
},

_arrangedContentWillChange: _beforeObserver('arrangedContent', function() {
let arrangedContent = get(this, 'arrangedContent');
let len = arrangedContent ? get(arrangedContent, 'length') : 0;

this.arrangedContentArrayWillChange(this, 0, len, undefined);
this.arrangedContentWillChange(this);

this._teardownArrangedContent(arrangedContent);
}),
Expand All @@ -214,18 +136,16 @@ export default EmberObject.extend(MutableArray, {
let arrangedContent = get(this, 'arrangedContent');
let len = arrangedContent ? get(arrangedContent, 'length') : 0;

assert('Can\'t set ArrayProxy\'s content to itself', arrangedContent !== this);

this._setupArrangedContent();

this.arrangedContentDidChange(this);
this.arrangedContentArrayDidChange(this, 0, undefined, len);
}),

_setupArrangedContent() {
let arrangedContent = get(this, 'arrangedContent');

if (arrangedContent) {
assert('Can\'t set ArrayProxy\'s content to itself', arrangedContent !== this);
assert(`ArrayProxy expects an Array or Ember.ArrayProxy, but you passed ${typeof arrangedContent}`,
isArray(arrangedContent) || arrangedContent.isDestroyed);

Expand All @@ -247,9 +167,6 @@ export default EmberObject.extend(MutableArray, {
}
},

arrangedContentWillChange: K,
arrangedContentDidChange: K,

objectAt(idx) {
return get(this, 'content') && this.objectAtContent(idx);
},
Expand Down Expand Up @@ -376,12 +293,10 @@ export default EmberObject.extend(MutableArray, {

init() {
this._super(...arguments);
this._setupContent();
this._setupArrangedContent();
},

willDestroy() {
this._teardownArrangedContent();
this._teardownContent();
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -35,54 +35,6 @@ QUnit.test('should update length for null content when there is a computed prope
assert.equal(proxy.get('length'), 0, 'length updates');
});

QUnit.test('The `arrangedContentWillChange` method is invoked before `content` is changed.', function(assert) {
let callCount = 0;
let expectedLength;

let proxy = ArrayProxy.extend({
arrangedContentWillChange() {
assert.equal(this.get('arrangedContent.length'), expectedLength, 'hook should be invoked before array has changed');
callCount++;
}
}).create({ content: emberA([1, 2, 3]) });

proxy.pushObject(4);
assert.equal(callCount, 0, 'pushing content onto the array doesn\'t trigger it');

proxy.get('content').pushObject(5);
assert.equal(callCount, 0, 'pushing content onto the content array doesn\'t trigger it');

expectedLength = 5;
proxy.set('content', emberA(['a', 'b']));
assert.equal(callCount, 1, 'replacing the content array triggers the hook');
});

QUnit.test('The `arrangedContentDidChange` method is invoked after `content` is changed.', function(assert) {
let callCount = 0;
let expectedLength;

let proxy = ArrayProxy.extend({
arrangedContentDidChange() {
assert.equal(this.get('arrangedContent.length'), expectedLength, 'hook should be invoked after array has changed');
callCount++;
}
}).create({
content: emberA([1, 2, 3])
});

assert.equal(callCount, 0, 'hook is not called after creating the object');

proxy.pushObject(4);
assert.equal(callCount, 0, 'pushing content onto the array doesn\'t trigger it');

proxy.get('content').pushObject(5);
assert.equal(callCount, 0, 'pushing content onto the content array doesn\'t trigger it');

expectedLength = 2;
proxy.set('content', emberA(['a', 'b']));
assert.equal(callCount, 1, 'replacing the content array triggers the hook');
});

QUnit.test('The ArrayProxy doesn\'t explode when assigned a destroyed object', function(assert) {
let proxy1 = ArrayProxy.create();
let proxy2 = ArrayProxy.create();
Expand Down Expand Up @@ -122,3 +74,21 @@ QUnit.test('arrayContent{Will,Did}Change are called when the content changes', f
assert.equal(willChangeCallCount, 2);
assert.equal(didChangeCallCount, 2);
});

QUnit.test('addArrayObserver works correctly', function(assert) {
let content = emberA([]);
let proxy = ArrayProxy.create({ content });

assert.expect(2);

proxy.addArrayObserver({
arrayWillChange(arr) {
assert.equal(arr.get('length'), 0);
},
arrayDidChange(arr) {
assert.equal(arr.get('length'), 3);
}
});

content.replace(0, 0, ['a', 'b', 'c']);
});

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ QUnit.test(`setting 'content' adds listeners correctly`, function(assert) {

assert.deepEqual(
sortedListenersFor(content, '@array:before'),
[[proxy, 'contentArrayWillChange'], [proxy, 'arrangedContentArrayWillChange']]
[[proxy, 'arrangedContentArrayWillChange']]
);
assert.deepEqual(
sortedListenersFor(content, '@array:change'),
[[proxy, 'contentArrayDidChange'], [proxy, 'arrangedContentArrayDidChange']]
[[proxy, 'arrangedContentArrayDidChange']]
);
});

Expand All @@ -43,11 +43,11 @@ QUnit.test(`changing 'content' adds and removes listeners correctly`, function(a

assert.deepEqual(
sortedListenersFor(content1, '@array:before'),
[[proxy, 'contentArrayWillChange'], [proxy, 'arrangedContentArrayWillChange']]
[[proxy, 'arrangedContentArrayWillChange']]
);
assert.deepEqual(
sortedListenersFor(content1, '@array:change'),
[[proxy, 'contentArrayDidChange'], [proxy, 'arrangedContentArrayDidChange']]
[[proxy, 'arrangedContentArrayDidChange']]
);

proxy.set('content', content2);
Expand All @@ -56,11 +56,11 @@ QUnit.test(`changing 'content' adds and removes listeners correctly`, function(a
assert.deepEqual(sortedListenersFor(content1, '@array:change'), []);
assert.deepEqual(
sortedListenersFor(content2, '@array:before'),
[[proxy, 'contentArrayWillChange'], [proxy, 'arrangedContentArrayWillChange']]
[[proxy, 'arrangedContentArrayWillChange']]
);
assert.deepEqual(
sortedListenersFor(content2, '@array:change'),
[[proxy, 'contentArrayDidChange'], [proxy, 'arrangedContentArrayDidChange']]
[[proxy, 'arrangedContentArrayDidChange']]
);
});

Expand Down

0 comments on commit 4d5ceed

Please sign in to comment.