Skip to content

Commit 3c6cdb6

Browse files
NullVoxPopuliNullVoxPopuli
committed
ye
1 parent 12bcfcd commit 3c6cdb6

File tree

3 files changed

+123
-15
lines changed

3 files changed

+123
-15
lines changed

‎packages/@glimmer-workspace/integration-tests/lib/render-test.ts

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -431,13 +431,15 @@ export class RenderTest implements IRenderTest {
431431
inTransaction(result.env, () => destroy(result));
432432
}
433433

434-
private assertEachCompareResults(items) {
434+
private assertEachCompareResults(
435+
items: (number | string | [string | number, string | number])[]
436+
) {
435437
[...(this.element as unknown as HTMLElement).querySelectorAll('.test-item')].forEach(
436438
(el, index) => {
437439
let key = Array.isArray(items[index]) ? items[index][0] : index;
438440
let value = Array.isArray(items[index]) ? items[index][1] : items[index];
439441

440-
QUnit.assert.equal(el.innerText, `${key}.${value}`);
442+
QUnit.assert.equal(el.textContent, `${key}.${value}`);
441443
}
442444
);
443445
}
@@ -447,7 +449,7 @@ export class RenderTest implements IRenderTest {
447449
shouldUpdate = true,
448450
message?: string
449451
) {
450-
let instance;
452+
let instance: TestComponent | undefined;
451453
let count = 0;
452454

453455
class TestComponent extends Klass {
@@ -477,6 +479,10 @@ export class RenderTest implements IRenderTest {
477479

478480
QUnit.assert.equal(count, 1, `The count is 1`);
479481

482+
if (!instance) {
483+
throw new Error('The instance is not defined');
484+
}
485+
480486
instance.update();
481487

482488
this.rerender();
@@ -493,10 +499,10 @@ export class RenderTest implements IRenderTest {
493499
protected assertEachInReactivity(
494500
Klass: new (...args: any[]) => { collection: number[]; update: () => void }
495501
) {
496-
let instance: TestComponent;
502+
let instance: TestComponent | undefined;
497503

498504
class TestComponent extends Klass {
499-
constructor(...args) {
505+
constructor(...args: unknown[]) {
500506
super(...args);
501507
// eslint-disable-next-line
502508
instance = this;
@@ -520,6 +526,10 @@ export class RenderTest implements IRenderTest {
520526

521527
this.renderComponent(comp);
522528

529+
if (!instance) {
530+
throw new Error('The instance is not defined');
531+
}
532+
523533
let { collection } = instance;
524534

525535
this.assertEachCompareResults(
@@ -538,7 +548,7 @@ export class RenderTest implements IRenderTest {
538548
protected assertEachReactivity(
539549
Klass: new (...args: any[]) => { collection: number[]; update: () => void }
540550
) {
541-
let instance: TestComponent;
551+
let instance: TestComponent | undefined;
542552

543553
class TestComponent extends Klass {
544554
constructor(...args: any[]) {
@@ -565,6 +575,10 @@ export class RenderTest implements IRenderTest {
565575

566576
this.renderComponent(comp);
567577

578+
if (!instance) {
579+
throw new Error('The instance is not defined');
580+
}
581+
568582
this.assertEachCompareResults(Array.from(instance.collection).map((v, i) => [i, v]));
569583

570584
instance.update();

‎packages/@glimmer-workspace/integration-tests/test/collections/array-test.ts

Lines changed: 93 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { trackedArray } from '@glimmer/validator';
21
import type { Dict, Owner } from '@glimmer/interfaces';
2+
import { trackedArray } from '@glimmer/validator';
33
import {
44
GlimmerishComponent as Component,
55
jitSuite,
@@ -181,6 +181,46 @@ class TrackedArrayTest extends RenderTest {
181181
`[[${method} individual index]]`
182182
);
183183

184+
this.assertReactivity(
185+
class extends Component {
186+
arr = trackedArray(['foo', 'bar'], { equals: () => false });
187+
188+
get value() {
189+
// @ts-expect-error -- this can't be represented easily in TS, and we
190+
// don't actually care that it is; we're *just* testing reactivity.
191+
return this.arr[method](() => {
192+
/* no op */
193+
});
194+
}
195+
196+
update() {
197+
this.arr[0] = 'bar';
198+
}
199+
},
200+
true,
201+
`[[${method} individual index via reference equality (no Object.is) -- tracked-built-ins behavior]]`
202+
);
203+
204+
this.assertReactivity(
205+
class extends Component {
206+
arr = trackedArray([{ foo: 'bar' }], { equals: (a, b) => a.foo === b.foo });
207+
208+
get value() {
209+
// @ts-expect-error -- this can't be represented easily in TS, and we
210+
// don't actually care that it is; we're *just* testing reactivity.
211+
return this.arr[method](() => {
212+
/* no op */
213+
});
214+
}
215+
216+
update() {
217+
this.arr[0] = { foo: 'bar' };
218+
}
219+
},
220+
false,
221+
`[[${method} no-op assign via custom equals]]`
222+
);
223+
184224
this.assertReactivity(
185225
class extends Component {
186226
arr = trackedArray(['foo', 'bar']);
@@ -203,12 +243,60 @@ class TrackedArrayTest extends RenderTest {
203243
});
204244
}
205245

246+
@test
247+
'Mutating collection set methods + default equals'() {
248+
['fill', 'pop', 'push', 'sort', 'unshift', 'splice'].forEach((method) => {
249+
this.assertReactivity(
250+
class extends Component {
251+
arr = trackedArray(['foo', 'bar']);
252+
253+
get value() {
254+
return void this.arr.forEach(() => {
255+
/* no op */
256+
});
257+
}
258+
259+
update() {
260+
// @ts-expect-error -- this can't be represented easily in TS, and we
261+
// don't actually care that it is; we're *just* testing reactivity.
262+
this.arr[method](undefined);
263+
}
264+
},
265+
true,
266+
`[[${method} collection tag]]`
267+
);
268+
});
269+
}
270+
271+
@test
272+
'copyWithin with no arguments does not cause a change'() {
273+
this.assertReactivity(
274+
class extends Component {
275+
arr = trackedArray(['foo', 'bar']);
276+
277+
get value() {
278+
return void this.arr.forEach(() => {
279+
/* no op */
280+
});
281+
}
282+
283+
update() {
284+
// @ts-expect-error -- this can't be represented easily in TS, and we
285+
// don't actually care that it is; we're *just* testing reactivity.
286+
this.arr.copyWithin(undefined);
287+
}
288+
},
289+
false,
290+
`[[copyWithin with no args collection tag]]`
291+
);
292+
}
293+
206294
@test
207295
ARRAY_SETTER_METHODS() {
208296
ARRAY_SETTER_METHODS.forEach((method) => {
209297
this.assertReactivity(
210298
class extends Component {
211-
arr = trackedArray(['foo', 'bar']);
299+
arr = trackedArray(['foo', 'bar'], { equals: () => false });
212300

213301
get value() {
214302
return this.arr[0];
@@ -221,12 +309,12 @@ class TrackedArrayTest extends RenderTest {
221309
}
222310
},
223311
true,
224-
`[[${method} individual index]]`
312+
`[[${method} individual index via reference equality (no Object.is) -- tracked-built-ins behavior]]`
225313
);
226314

227315
this.assertReactivity(
228316
class extends Component {
229-
arr = trackedArray(['foo', 'bar']);
317+
arr = trackedArray(['foo', 'bar'], { equals: () => false });
230318

231319
get value() {
232320
return void this.arr.forEach(() => {
@@ -241,8 +329,7 @@ class TrackedArrayTest extends RenderTest {
241329
}
242330
},
243331
true,
244-
245-
`[[${method} collection tag]]`
332+
`[[${method} collection tag via reference equality (no Object.is) -- tracked-built-ins behavior]]`
246333
);
247334
});
248335
}

‎packages/@glimmer/validator/lib/collections/array.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,12 @@ function convertToInt(prop: number | string | symbol): number | null {
4949

5050
// eslint-disable-next-line @typescript-eslint/no-unsafe-declaration-merging
5151
export class TrackedArray<T = unknown> {
52-
#options: { equals: (a: T, b: T) => boolean; description: string | undefined; };
52+
#options: { equals: (a: T, b: T) => boolean; description: string | undefined };
5353

54-
constructor(arr: T[], options: { equals: (a: T, b: T) => boolean; description: string | undefined }) {
54+
constructor(
55+
arr: T[],
56+
options: { equals: (a: T, b: T) => boolean; description: string | undefined }
57+
) {
5558
this.#options = options;
5659

5760
const clone = arr.slice();
@@ -124,7 +127,11 @@ export class TrackedArray<T = unknown> {
124127
},
125128

126129
set(target, prop, value /*, _receiver */) {
127-
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment
130+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-argument
131+
let isUnchanged = self.#options.equals((target as any)[prop], value);
132+
if (isUnchanged) return true;
133+
134+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access
128135
(target as any)[prop] = value;
129136

130137
const index = convertToInt(prop);

0 commit comments

Comments
 (0)