Skip to content

Commit

Permalink
[BUGFIX release] ArrayProxy should not be a computed property
Browse files Browse the repository at this point in the history
Fixes emberjs#12475.

This commit restores the contract of ArrayProxy content being a simple
property instead of a computed property. This prevents accidental
clobberings such as in ember-data:

https://github.com/emberjs/data/blob/v2.3.3/addon/-private/system/record-arrays/record-array.js#L44

This commit also removes a private hook `arrangedContentArrayWillChange`.
In the interest of keeping this bugfix commit minimal, we will continue
removing the will* hooks on canary.
  • Loading branch information
mmun committed Jan 24, 2016
1 parent 76e9c59 commit 15eb964
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 44 deletions.
31 changes: 9 additions & 22 deletions packages/ember-runtime/lib/system/array_proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,23 +74,7 @@ var ArrayProxy = EmberObject.extend(MutableArray, {
@type Ember.Array
@private
*/
content: computed({
get() {
return this._content;
},
set(k, v) {
if (this._didInitArrayProxy) {
var oldContent = this._content;
var len = oldContent ? get(oldContent, 'length') : 0;
this.arrangedContentArrayWillChange(this, 0, len, undefined);
this.arrangedContentWillChange(this);
}
this._content = v;
return v;
}
}),


content: null,

/**
The array that the proxy pretends to be. In the default `ArrayProxy`
Expand Down Expand Up @@ -179,12 +163,16 @@ var ArrayProxy = EmberObject.extend(MutableArray, {
@method _contentDidChange
*/
_contentDidChange: observer('content', function() {
var content = get(this, 'content');
this._teardownContent(this._prevContent);
let prevContent = this._prevContent;
let content = get(this, 'content');

assert('Can\'t set ArrayProxy\'s content to itself', content !== this);
if (content !== prevContent) {
this._teardownContent(prevContent);

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

this._setupContent();
}
}),

_setupContent() {
Expand Down Expand Up @@ -369,7 +357,6 @@ var ArrayProxy = EmberObject.extend(MutableArray, {
},

init() {
this._didInitArrayProxy = true;
this._super(...arguments);
this._setupContent();
this._setupArrangedContent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,28 +36,6 @@ QUnit.test('should update length for null content when there is a computed prope
equal(proxy.get('length'), 0, 'length updates');
});

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

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

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

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

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

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

0 comments on commit 15eb964

Please sign in to comment.