Skip to content

Commit

Permalink
[BUGFIX lts] Entangles custom EmberArray implementations when accessed
Browse files Browse the repository at this point in the history
Restores the previous logic that existed for entangling custom
EmberArray implementations when they are accessed, along with a test
for the functionality.
  • Loading branch information
Chris Garrett committed Sep 28, 2020
1 parent 984e69c commit 48f6901
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 15 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { Object as EmberObject, A } from '@ember/-internals/runtime';
import { get, set, tracked, nativeDescDecorator as descriptor } from '@ember/-internals/metal';
import { Object as EmberObject, A, MutableArray } from '@ember/-internals/runtime';
import {
get,
set,
tracked,
nativeDescDecorator as descriptor,
notifyPropertyChange,
} from '@ember/-internals/metal';
import Service, { inject } from '@ember/service';
import { moduleFor, RenderingTestCase, strip, runTask } from 'internal-test-helpers';

Expand Down Expand Up @@ -169,6 +175,61 @@ moduleFor(
this.assertText('1, 2, 3, 4');
}

'@test custom ember array properties rerender when updated'() {
let CustomArray = EmberObject.extend(MutableArray, {
init() {
this._super(...arguments);
this._vals = [1, 2, 3];
},

objectAt(index) {
return this._vals[index];
},

replace(start, deleteCount, items = []) {
this._vals.splice(start, deleteCount, ...items);
notifyPropertyChange(this, '[]');
},

join() {
return this._vals.join(...arguments);
},

get length() {
return this._vals.length;
},
});

let NumListComponent = Component.extend({
numbers: tracked({ initializer: () => CustomArray.create() }),

addNumber() {
this.numbers.pushObject(4);
},
});

this.registerComponent('num-list', {
ComponentClass: NumListComponent,
template: strip`
<button {{action this.addNumber}}>
{{join this.numbers}}
</button>
`,
});

this.registerHelper('join', ([value]) => {
return value.join(', ');
});

this.render('<NumList />');

this.assertText('1, 2, 3');

runTask(() => this.$('button').click());

this.assertText('1, 2, 3, 4');
}

'@test nested getters update when dependent properties are invalidated'(assert) {
let computeCount = 0;

Expand Down
4 changes: 2 additions & 2 deletions packages/@ember/-internals/metal/lib/property_get.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
@module @ember/object
*/
import { HAS_NATIVE_PROXY, setProxy, symbol } from '@ember/-internals/utils';
import { HAS_NATIVE_PROXY, isEmberArray, setProxy, symbol } from '@ember/-internals/utils';
import { assert, deprecate } from '@ember/debug';
import { DEBUG } from '@glimmer/env';
import {
Expand Down Expand Up @@ -130,7 +130,7 @@ export function _getProp(obj: object, keyName: string) {
if (isTracking()) {
consumeTag(tagFor(obj, keyName));

if (Array.isArray(value)) {
if (Array.isArray(value) || isEmberArray(value)) {
// Add the tag of the returned value if it is an array, since arrays
// should always cause updates if they are consumed and then changed
consumeTag(tagFor(value, '[]'));
Expand Down
3 changes: 2 additions & 1 deletion packages/@ember/-internals/metal/lib/tracked.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { isEmberArray } from '@ember/-internals/utils';
import { assert } from '@ember/debug';
import { DEBUG } from '@glimmer/env';
import { consumeTag, dirtyTagFor, tagFor, trackedData } from '@glimmer/validator';
Expand Down Expand Up @@ -162,7 +163,7 @@ function descriptorForField([_target, key, desc]: [

// Add the tag of the returned value if it is an array, since arrays
// should always cause updates if they are consumed and then changed
if (Array.isArray(value)) {
if (Array.isArray(value) || isEmberArray(value)) {
consumeTag(tagFor(value, '[]'));
}

Expand Down
8 changes: 6 additions & 2 deletions packages/@ember/-internals/runtime/lib/mixins/array.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/
import { DEBUG } from '@glimmer/env';
import { PROXY_CONTENT } from '@ember/-internals/metal';
import { EMBER_ARRAY, HAS_NATIVE_PROXY, tryInvoke } from '@ember/-internals/utils';
import { setEmberArray, HAS_NATIVE_PROXY, tryInvoke } from '@ember/-internals/utils';
import {
get,
set,
Expand Down Expand Up @@ -216,7 +216,11 @@ function mapBy(key) {
@public
*/
const ArrayMixin = Mixin.create(Enumerable, {
[EMBER_ARRAY]: true,
init() {
this._super(...arguments);
setEmberArray(this);
},


/**
__Required.__ You must implement this method to apply this mixin.
Expand Down
2 changes: 1 addition & 1 deletion packages/@ember/-internals/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export { HAS_NATIVE_SYMBOL } from './lib/symbol-utils';
export { HAS_NATIVE_PROXY } from './lib/proxy-utils';
export { isProxy, setProxy } from './lib/is_proxy';
export { default as Cache } from './lib/cache';
export { EMBER_ARRAY, EmberArray, isEmberArray } from './lib/ember-array';
export { EmberArray, setEmberArray, isEmberArray } from './lib/ember-array';
export {
setupMandatorySetter,
teardownMandatorySetter,
Expand Down
12 changes: 8 additions & 4 deletions packages/@ember/-internals/utils/lib/ember-array.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { symbol } from './symbol';
import { _WeakSet } from '@glimmer/util';

export const EMBER_ARRAY = symbol('EMBER_ARRAY');
const EMBER_ARRAYS = new _WeakSet();

export interface EmberArray<T> {
length: number;
Expand All @@ -10,6 +10,10 @@ export interface EmberArray<T> {
splice(start: number, deleteCount: number, ...items: T[]): void;
}

export function isEmberArray(obj: any): obj is EmberArray<unknown> {
return obj && obj[EMBER_ARRAY];
export function setEmberArray(obj: object) {
EMBER_ARRAYS.add(obj);
}

export function isEmberArray(obj: unknown): obj is EmberArray<unknown> {
return EMBER_ARRAYS.has(obj as object);
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import { EMBER_ARRAY, isEmberArray } from '..';
import { setEmberArray, isEmberArray } from '..';
import { moduleFor, AbstractTestCase } from 'internal-test-helpers';

moduleFor(
'@ember/-internals/utils Trackable Object',
class extends AbstractTestCase {
['@test classes'](assert) {
class Test {}
Test.prototype[EMBER_ARRAY] = true;
class Test {
constructor() {
setEmberArray(this);
}
}

let instance = new Test();

Expand Down

0 comments on commit 48f6901

Please sign in to comment.