Skip to content

Commit ade1ca6

Browse files
rwjbluerwjblue
authored
Merge pull request #13534 from btecu/removeAt
[BUGFIX beta] Don't use prototype extensions (arrayContent{Did,Will}Change, removeAt)
2 parents fd69da5 + 5f5407c commit ade1ca6

File tree

12 files changed

+213
-162
lines changed

12 files changed

+213
-162
lines changed

‎packages/ember-glimmer/tests/integration/helpers/element-action-test.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { set } from 'ember-metal/property_set';
55

66
import EmberObject from 'ember-runtime/system/object';
77
import { A as emberA } from 'ember-runtime/system/native_array';
8+
import { removeAt } from 'ember-runtime/mixins/mutable_array';
89

910
import ActionManager from 'ember-views/system/action_manager';
1011
import jQuery from 'ember-views/system/jquery';
@@ -761,7 +762,7 @@ moduleFor('Helpers test: element action', class extends RenderingTest {
761762
var actionId = this.$('a[data-ember-action]').attr('data-ember-action');
762763

763764
this.runTask(() => {
764-
things.removeAt(0);
765+
removeAt(things, 0);
765766
});
766767

767768
ok(!ActionManager.registeredActions[actionId], 'After the virtual view was destroyed, the action was unregistered');

‎packages/ember-glimmer/tests/integration/syntax/each-test.js‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { set } from 'ember-metal/property_set';
33
import { applyMixins, strip } from '../../utils/abstract-test-case';
44
import { moduleFor, RenderingTest } from '../../utils/test-case';
55
import { A as emberA } from 'ember-runtime/system/native_array';
6+
import { removeAt } from 'ember-runtime/mixins/mutable_array';
67

78
import {
89
BasicConditionalsTest,
@@ -78,7 +79,7 @@ moduleFor('Syntax test: {{#each as}}', class extends EachTest {
7879
this.runTask(() => {
7980
let list = get(this.context, 'list');
8081
list.pushObject({ text: 'Earth' });
81-
list.removeAt(1);
82+
removeAt(list, 1);
8283
list.insertAt(1, { text: 'Globe' });
8384
});
8485

@@ -87,23 +88,23 @@ moduleFor('Syntax test: {{#each as}}', class extends EachTest {
8788
this.runTask(() => {
8889
let list = get(this.context, 'list');
8990
list.pushObject({ text: 'Planet' });
90-
list.removeAt(1);
91+
removeAt(list, 1);
9192
list.insertAt(1, { text: ' ' });
9293
list.pushObject({ text: ' ' });
9394
list.pushObject({ text: 'Earth' });
94-
list.removeAt(3);
95+
removeAt(list, 3);
9596
});
9697

9798
this.assertText('Hello WorldPlanet Earth');
9899

99100
this.runTask(() => {
100101
let list = get(this.context, 'list');
101102
list.pushObject({ text: 'Globe' });
102-
list.removeAt(1);
103+
removeAt(list, 1);
103104
list.insertAt(1, { text: ' ' });
104105
list.pushObject({ text: ' ' });
105106
list.pushObject({ text: 'World' });
106-
list.removeAt(2);
107+
removeAt(list, 2);
107108
});
108109

109110
this.assertText('Hello Planet EarthGlobe World');
@@ -372,7 +373,7 @@ moduleFor('Syntax test: {{#each as}}', class extends EachTest {
372373
this.runTask(() => {
373374
let people = get(this.context, 'people');
374375
set(people.objectAt(1), 'name', 'Stefan Penner');
375-
people.removeAt(0);
376+
removeAt(people, 0);
376377
people.pushObject({ name: 'Tom Dale' });
377378
people.insertAt(1, { name: 'Chad Hietala' });
378379
set(this.context, 'title', 'Principal Engineer');

‎packages/ember-glimmer/tests/integration/syntax/with-test.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { moduleFor, RenderingTest } from '../../utils/test-case';
55
import { TogglingSyntaxConditionalsTest } from '../../utils/shared-conditional-tests';
66
import { strip } from '../../utils/abstract-test-case';
77
import ObjectProxy from 'ember-runtime/system/object_proxy';
8+
import { removeAt } from 'ember-runtime/mixins/mutable_array';
89

910
moduleFor('Syntax test: {{#with}}', class extends TogglingSyntaxConditionalsTest {
1011

@@ -198,7 +199,7 @@ moduleFor('Syntax test: {{#with as}}', class extends TogglingSyntaxConditionalsT
198199
this.runTask(() => {
199200
let array = get(this.context, 'arrayThing');
200201
array.replace(0, 1, 'Goodbye');
201-
array.removeAt(1);
202+
removeAt(array, 1);
202203
array.insertAt(1, ', ');
203204
array.pushObject('!');
204205
});

‎packages/ember-glimmer/tests/utils/shared-conditional-tests.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import EmberObject from 'ember-runtime/system/object';
77
import ObjectProxy from 'ember-runtime/system/object_proxy';
88
import { A as emberA } from 'ember-runtime/system/native_array';
99
import ArrayProxy from 'ember-runtime/system/array_proxy';
10+
import { removeAt } from 'ember-runtime/mixins/mutable_array';
1011
import { Component } from './helpers';
1112

1213
class AbstractConditionalsTest extends RenderingTest {
@@ -308,7 +309,7 @@ export const ArrayTestCases = {
308309

309310
this.assertText('T1F2');
310311

311-
this.runTask(() => get(this.context, 'cond1').removeAt(0));
312+
this.runTask(() => removeAt(get(this.context, 'cond1'), 0));
312313

313314
this.assertText('F1F2');
314315

@@ -373,7 +374,7 @@ export const ArrayTestCases = {
373374

374375
this.assertText('T1F2');
375376

376-
this.runTask(() => get(this.context, 'cond1.content').removeAt(0));
377+
this.runTask(() => removeAt(get(this.context, 'cond1.content'), 0));
377378

378379
this.assertText('F1F2');
379380

‎packages/ember-runtime/lib/mixins/array.js‎

Lines changed: 95 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,99 @@ export function objectAt(content, idx) {
6868
return content[idx];
6969
}
7070

71+
export function arrayContentWillChange(array, startIdx, removeAmt, addAmt) {
72+
let removing, lim;
73+
74+
// if no args are passed assume everything changes
75+
if (startIdx === undefined) {
76+
startIdx = 0;
77+
removeAmt = addAmt = -1;
78+
} else {
79+
if (removeAmt === undefined) {
80+
removeAmt = -1;
81+
}
82+
83+
if (addAmt === undefined) {
84+
addAmt = -1;
85+
}
86+
}
87+
88+
if (array.__each) {
89+
array.__each.arrayWillChange(array, startIdx, removeAmt, addAmt);
90+
}
91+
92+
sendEvent(array, '@array:before', [array, startIdx, removeAmt, addAmt]);
93+
94+
if (startIdx >= 0 && removeAmt >= 0 && get(array, 'hasEnumerableObservers')) {
95+
removing = [];
96+
lim = startIdx + removeAmt;
97+
98+
for (let idx = startIdx; idx < lim; idx++) {
99+
removing.push(objectAt(array, idx));
100+
}
101+
} else {
102+
removing = removeAmt;
103+
}
104+
105+
array.enumerableContentWillChange(removing, addAmt);
106+
107+
return array;
108+
}
109+
110+
export function arrayContentDidChange(array, startIdx, removeAmt, addAmt) {
111+
markObjectAsDirty(metaFor(array));
112+
113+
// if no args are passed assume everything changes
114+
if (startIdx === undefined) {
115+
startIdx = 0;
116+
removeAmt = addAmt = -1;
117+
} else {
118+
if (removeAmt === undefined) {
119+
removeAmt = -1;
120+
}
121+
122+
if (addAmt === undefined) {
123+
addAmt = -1;
124+
}
125+
}
126+
127+
let adding;
128+
if (startIdx >= 0 && addAmt >= 0 && get(array, 'hasEnumerableObservers')) {
129+
adding = [];
130+
let lim = startIdx + addAmt;
131+
132+
for (let idx = startIdx; idx < lim; idx++) {
133+
adding.push(objectAt(array, idx));
134+
}
135+
} else {
136+
adding = addAmt;
137+
}
138+
139+
array.enumerableContentDidChange(removeAmt, adding);
140+
141+
if (array.__each) {
142+
array.__each.arrayDidChange(array, startIdx, removeAmt, addAmt);
143+
}
144+
145+
sendEvent(array, '@array:change', [array, startIdx, removeAmt, addAmt]);
146+
147+
let length = get(array, 'length');
148+
let cachedFirst = cacheFor(array, 'firstObject');
149+
let cachedLast = cacheFor(array, 'lastObject');
150+
151+
if (objectAt(array, 0) !== cachedFirst) {
152+
propertyWillChange(array, 'firstObject');
153+
propertyDidChange(array, 'firstObject');
154+
}
155+
156+
if (objectAt(array, length - 1) !== cachedLast) {
157+
propertyWillChange(array, 'lastObject');
158+
propertyDidChange(array, 'lastObject');
159+
}
160+
161+
return array;
162+
}
163+
71164
const EMBER_ARRAY = symbol('EMBER_ARRAY');
72165

73166
export function isEmberArray(obj) {
@@ -437,43 +530,7 @@ const ArrayMixin = Mixin.create(Enumerable, {
437530
@public
438531
*/
439532
arrayContentWillChange(startIdx, removeAmt, addAmt) {
440-
let removing, lim;
441-
442-
// if no args are passed assume everything changes
443-
if (startIdx === undefined) {
444-
startIdx = 0;
445-
removeAmt = addAmt = -1;
446-
} else {
447-
if (removeAmt === undefined) {
448-
removeAmt = -1;
449-
}
450-
451-
if (addAmt === undefined) {
452-
addAmt = -1;
453-
}
454-
}
455-
456-
if (this.__each) {
457-
this.__each.arrayWillChange(this, startIdx, removeAmt, addAmt);
458-
}
459-
460-
sendEvent(this, '@array:before', [this, startIdx, removeAmt, addAmt]);
461-
462-
463-
if (startIdx >= 0 && removeAmt >= 0 && get(this, 'hasEnumerableObservers')) {
464-
removing = [];
465-
lim = startIdx + removeAmt;
466-
467-
for (let idx = startIdx; idx < lim; idx++) {
468-
removing.push(objectAt(this, idx));
469-
}
470-
} else {
471-
removing = removeAmt;
472-
}
473-
474-
this.enumerableContentWillChange(removing, addAmt);
475-
476-
return this;
533+
return arrayContentWillChange(this, startIdx, removeAmt, addAmt);
477534
},
478535

479536
/**
@@ -492,57 +549,7 @@ const ArrayMixin = Mixin.create(Enumerable, {
492549
@public
493550
*/
494551
arrayContentDidChange(startIdx, removeAmt, addAmt) {
495-
markObjectAsDirty(metaFor(this));
496-
497-
// if no args are passed assume everything changes
498-
if (startIdx === undefined) {
499-
startIdx = 0;
500-
removeAmt = addAmt = -1;
501-
} else {
502-
if (removeAmt === undefined) {
503-
removeAmt = -1;
504-
}
505-
506-
if (addAmt === undefined) {
507-
addAmt = -1;
508-
}
509-
}
510-
511-
let adding;
512-
if (startIdx >= 0 && addAmt >= 0 && get(this, 'hasEnumerableObservers')) {
513-
adding = [];
514-
let lim = startIdx + addAmt;
515-
516-
for (let idx = startIdx; idx < lim; idx++) {
517-
adding.push(objectAt(this, idx));
518-
}
519-
} else {
520-
adding = addAmt;
521-
}
522-
523-
this.enumerableContentDidChange(removeAmt, adding);
524-
525-
if (this.__each) {
526-
this.__each.arrayDidChange(this, startIdx, removeAmt, addAmt);
527-
}
528-
529-
sendEvent(this, '@array:change', [this, startIdx, removeAmt, addAmt]);
530-
531-
let length = get(this, 'length');
532-
let cachedFirst = cacheFor(this, 'firstObject');
533-
let cachedLast = cacheFor(this, 'lastObject');
534-
535-
if (objectAt(this, 0) !== cachedFirst) {
536-
propertyWillChange(this, 'firstObject');
537-
propertyDidChange(this, 'firstObject');
538-
}
539-
540-
if (objectAt(this, length - 1) !== cachedLast) {
541-
propertyWillChange(this, 'lastObject');
542-
propertyDidChange(this, 'lastObject');
543-
}
544-
545-
return this;
552+
return arrayContentDidChange(this, startIdx, removeAmt, addAmt);
546553
},
547554

548555
/**

‎packages/ember-runtime/lib/mixins/mutable_array.js‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,23 @@ import MutableEnumerable from 'ember-runtime/mixins/mutable_enumerable';
2626
import Enumerable from 'ember-runtime/mixins/enumerable';
2727
import isEnabled from 'ember-metal/features';
2828

29+
export function removeAt(array, start, len) {
30+
if ('number' === typeof start) {
31+
if ((start < 0) || (start >= get(array, 'length'))) {
32+
throw new EmberError(OUT_OF_RANGE_EXCEPTION);
33+
}
34+
35+
// fast case
36+
if (len === undefined) {
37+
len = 1;
38+
}
39+
40+
array.replace(start, len, EMPTY);
41+
}
42+
43+
return array;
44+
}
45+
2946
/**
3047
This mixin defines the API for modifying array-like objects. These methods
3148
can be applied only to a collection that keeps its items in an ordered set.
@@ -141,20 +158,7 @@ export default Mixin.create(EmberArray, MutableEnumerable, {
141158
@public
142159
*/
143160
removeAt(start, len) {
144-
if ('number' === typeof start) {
145-
if ((start < 0) || (start >= get(this, 'length'))) {
146-
throw new EmberError(OUT_OF_RANGE_EXCEPTION);
147-
}
148-
149-
// fast case
150-
if (len === undefined) {
151-
len = 1;
152-
}
153-
154-
this.replace(start, len, EMPTY);
155-
}
156-
157-
return this;
161+
return removeAt(this, start, len);
158162
},
159163

160164
/**

‎packages/ember-runtime/lib/system/array_proxy.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import alias from 'ember-metal/alias';
2020
import {
2121
addArrayObserver,
2222
removeArrayObserver,
23+
arrayContentDidChange,
24+
arrayContentWillChange,
2325
objectAt
2426
} from 'ember-runtime/mixins/array';
2527

@@ -33,6 +35,7 @@ const EMPTY = [];
3335

3436
function K() { return this; }
3537

38+
3639
/**
3740
An ArrayProxy wraps any other object that implements `Ember.Array` and/or
3841
`Ember.MutableArray,` forwarding all requests. This makes it very useful for
@@ -371,11 +374,11 @@ export default EmberObject.extend(MutableArray, {
371374
},
372375

373376
arrangedContentArrayWillChange(item, idx, removedCnt, addedCnt) {
374-
this.arrayContentWillChange(idx, removedCnt, addedCnt);
377+
arrayContentWillChange(this, idx, removedCnt, addedCnt);
375378
},
376379

377380
arrangedContentArrayDidChange(item, idx, removedCnt, addedCnt) {
378-
this.arrayContentDidChange(idx, removedCnt, addedCnt);
381+
arrayContentDidChange(this, idx, removedCnt, addedCnt);
379382
},
380383

381384
init() {

0 commit comments

Comments
 (0)