Skip to content

Commit

Permalink
[BUGFIX] Setting ArrayProxy#content in willDestroy resets length.
Browse files Browse the repository at this point in the history
Prior to this change, `ArrayProxy.prototype.length` was marked as dirty
(and therefore recomputed) when `content` was set due to
`arrangedContent` being an alias of `content`, and `ArrayProxy`
implementing `PROPERTY_DID_CHANGE` to trap `arrangedContent` changes.

Unfortunately, `notifyPropertyChange` avoids notifying _some_ things
when the object is destroying. This results in the preexisting
`PROPERTY_DID_CHANGE` trap for `arrangedContent` is no longer called,
and therefore `length` is never marked as dirty.

This fixes the condition, by implementing a `content` trap in
`PROPERTY_DID_CHANGE` that marks length as dirty. The next time that
`length` is accessed it will do the normal work to recalculate.
  • Loading branch information
rwjblue committed Jun 28, 2018
1 parent c714f6e commit 7189bf0
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 5 deletions.
14 changes: 9 additions & 5 deletions packages/ember-runtime/lib/system/array_proxy.js
Expand Up @@ -210,9 +210,7 @@ export default class ArrayProxy extends EmberObject {
if (content) {
replace(content, value, removedCount, added);

this._lengthDirty = true;
this._objectsDirtyIndex = 0;
this._objects = null;
this._invalidate();
}
}

Expand All @@ -225,11 +223,12 @@ export default class ArrayProxy extends EmberObject {
this._removeArrangedContentArrayObsever();
this.arrayContentWillChange(0, oldLength, newLength);

this._objectsDirtyIndex = 0;
this._lengthDirty = true;
this._invalidate();

this.arrayContentDidChange(0, oldLength, newLength);
this._addArrangedContentArrayObsever();
} else if (key === 'content') {
this._invalidate();
}
}

Expand Down Expand Up @@ -277,6 +276,11 @@ export default class ArrayProxy extends EmberObject {

this.arrayContentDidChange(idx, removedCnt, addedCnt);
}

_invalidate() {
this._objectsDirtyIndex = 0;
this._lengthDirty = true;
}
}

ArrayProxy.reopen(MutableArray, {
Expand Down
18 changes: 18 additions & 0 deletions packages/ember-runtime/tests/system/array_proxy/length_test.js
Expand Up @@ -78,6 +78,24 @@ moduleFor(
assert.deepEqual(obj.content, null, 'content was updated');
}

'@test accessing length after content set to null in willDestroy'(assert) {
let obj = ArrayProxy.extend({
willDestroy() {
this.set('content', null);
this._super(...arguments);
},
}).create({
content: ['foo', 'bar'],
});

assert.equal(obj.length, 2, 'precond');

this.runTask(() => obj.destroy());

assert.equal(obj.length, 0, 'length is 0 without content');
assert.deepEqual(obj.content, null, 'content was updated');
}

'@test setting length to 0'(assert) {
let obj = ArrayProxy.create({ content: ['foo', 'bar'] });

Expand Down

0 comments on commit 7189bf0

Please sign in to comment.