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.

Sign up for GitHub

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 legacy ArrayProxy features #16101

Merged
merged 4 commits into from
Jan 10, 2018
Merged

Remove legacy ArrayProxy features #16101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7a2ec1b
Remove private ArrayProxy contentArray{Will,Did}Change hooks
mmun Jan 10, 2018
30f66e5
Remove undocumented ArrayProxy arrangedContent{Will,Did}Change hooks
mmun Jan 10, 2018
3c48368
Add addArrayObserver sanity test for ArrayProxy
mmun Jan 10, 2018
78f94cb
ArrayProxy's content and arrangedContent should be @public
mmun Jan 10, 2018
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
91 changes: 3 additions & 88 deletions packages/ember-runtime/lib/system/array_proxy.js
View file
Open in desktop
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();
}
});
66 changes: 18 additions & 48 deletions packages/ember-runtime/tests/system/array_proxy/content_change_test.js
View file
Open in desktop
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']);
});
25 changes: 0 additions & 25 deletions packages/ember-runtime/tests/system/array_proxy/content_update_test.js
View file
Open in desktop

This file was deleted.

12 changes: 6 additions & 6 deletions packages/ember-runtime/tests/system/array_proxy/watching_and_listening_test.js
View file
Open in desktop
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