Skip to content

Commit

Permalink
Merge pull request #16157 from emberjs/array-proxy-no-mutate-arranged
Browse files Browse the repository at this point in the history
[BUGFIX beta] Mutating an arranged ArrayProxy is not allowed
  • Loading branch information
mmun authored Jan 21, 2018
2 parents 4c4d2b1 + bfc5a68 commit f2c2fb9
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 246 deletions.
13 changes: 0 additions & 13 deletions packages/ember-runtime/lib/mixins/array.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,19 +270,6 @@ const ArrayMixin = Mixin.create(Enumerable, {
return indexes.map(idx => objectAt(this, idx));
},

/**
@method nextObject
@param {Number} index the current index of the iteration
@param {Object} previousObject the value returned by the last call to
`nextObject`.
@param {Object} context a context object you can use to maintain state.
@return {Object} the next object in the iteration or undefined
@private
*/
nextObject(idx) {
return objectAt(this, idx);
},

/**
This is the handler for the special array content property. If you get
this property, it will return this. If you set this property to a new
Expand Down
123 changes: 8 additions & 115 deletions packages/ember-runtime/lib/system/array_proxy.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
/**
@module @ember/array
*/

import {
get,
computed,
_beforeObserver,
observer,
beginPropertyChanges,
endPropertyChanges,
alias
} from 'ember-metal';
import {
Expand All @@ -17,15 +19,7 @@ import {
removeArrayObserver,
objectAt
} from '../mixins/array';
import { assert, Error as EmberError } from 'ember-debug';

/**
@module @ember/array
*/

const OUT_OF_RANGE_EXCEPTION = 'Index out of range';
const EMPTY = [];

import { assert } from 'ember-debug';

/**
An ArrayProxy wraps any other object that implements `Array` and/or
Expand Down Expand Up @@ -182,110 +176,9 @@ export default EmberObject.extend(MutableArray, {
// No dependencies since Enumerable notifies length of change
}),

_replace(idx, amt, objects) {
let content = get(this, 'content');
assert(`The content property of ${this.constructor} should be set before modifying it`, content);
if (content) {
this.replaceContent(idx, amt, objects);
}

return this;
},

replace() {
if (get(this, 'arrangedContent') === get(this, 'content')) {
this._replace(...arguments);
} else {
throw new EmberError('Using replace on an arranged ArrayProxy is not allowed.');
}
},

_insertAt(idx, object) {
if (idx > get(this, 'content.length')) {
throw new EmberError(OUT_OF_RANGE_EXCEPTION);
}

this._replace(idx, 0, [object]);
return this;
},

insertAt(idx, object) {
if (get(this, 'arrangedContent') === get(this, 'content')) {
return this._insertAt(idx, object);
} else {
throw new EmberError('Using insertAt on an arranged ArrayProxy is not allowed.');
}
},

removeAt(start, len) {
if ('number' === typeof start) {
let content = get(this, 'content');
let arrangedContent = get(this, 'arrangedContent');
let indices = [];

if ((start < 0) || (start >= get(this, 'length'))) {
throw new EmberError(OUT_OF_RANGE_EXCEPTION);
}

if (len === undefined) {
len = 1;
}

// Get a list of indices in original content to remove
for (let i = start; i < start + len; i++) {
// Use arrangedContent here so we avoid confusion with objects transformed by objectAtContent
indices.push(content.indexOf(objectAt(arrangedContent, i)));
}

// Replace in reverse order since indices will change
indices.sort((a, b) => b - a);

beginPropertyChanges();
for (let i = 0; i < indices.length; i++) {
this._replace(indices[i], 1, EMPTY);
}
endPropertyChanges();
}

return this;
},

pushObject(obj) {
this._insertAt(get(this, 'content.length'), obj);
return obj;
},

pushObjects(objects) {
if (!isArray(objects)) {
throw new TypeError('Must pass Enumerable to MutableArray#pushObjects');
}
this._replace(get(this, 'length'), 0, objects);
return this;
},

setObjects(objects) {
if (objects.length === 0) {
return this.clear();
}

let len = get(this, 'length');
this._replace(0, len, objects);
return this;
},

unshiftObject(obj) {
this._insertAt(0, obj);
return obj;
},

unshiftObjects(objects) {
this._replace(0, 0, objects);
return this;
},

slice() {
let arr = this.toArray();
return arr.slice(...arguments);
replace(idx, amt, objects) {
assert('Mutating an arranged ArrayProxy is not allowed', get(this, 'arrangedContent') === get(this, 'content') );
this.replaceContent(idx, amt, objects);
},

arrangedContentArrayWillChange(item, idx, removedCnt, addedCnt) {
Expand Down
1 change: 0 additions & 1 deletion packages/ember-runtime/tests/mixins/array_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ QUnit.test('slice supports negative index arguments', function(assert) {
//

const DummyArray = EmberObject.extend(EmberArray, {
nextObject() {},
length: 0,
objectAt(idx) { return 'ITEM-' + idx; }
});
Expand Down
Loading

0 comments on commit f2c2fb9

Please sign in to comment.