Skip to content

Commit 2c5ff30

Browse files
authored
Merge pull request #21139 from emberjs/copilot/port-bugfix-destroyable-meta
Port BrandedArray fix for Array-as-parent bug in destroyables (glimmer-vm#1771)
2 parents e9c61cc + 5859b78 commit 2c5ff30

2 files changed

Lines changed: 25 additions & 6 deletions

File tree

packages/@glimmer/destroyable/index.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const DESTROYING_STATE = 1;
88
const DESTROYED_STATE = 2;
99
type DestroyableState = 0 | 1 | 2;
1010

11-
type OneOrMany<T> = null | T | T[];
11+
type OneOrMany<T> = null | T | BrandedArray<T>;
1212

1313
interface DestroyableMeta<T extends Destroyable> {
1414
source?: T;
@@ -27,19 +27,28 @@ let DESTROYABLE_META:
2727
| Map<Destroyable, DestroyableMeta<Destroyable>>
2828
| WeakMap<Destroyable, DestroyableMeta<Destroyable>> = new WeakMap();
2929

30+
const branded = Symbol('BrandedArray');
31+
type BrandedArray<T> = T[] & { [branded]: true };
32+
33+
function isBrandedArray<T>(collection: OneOrMany<T>): collection is BrandedArray<T> {
34+
return Array.isArray(collection) && branded in collection;
35+
}
36+
3037
function push<T extends object>(collection: OneOrMany<T>, newItem: T): OneOrMany<T> {
3138
if (collection === null) {
3239
return newItem;
33-
} else if (Array.isArray(collection)) {
40+
} else if (isBrandedArray(collection)) {
3441
collection.push(newItem);
3542
return collection;
3643
} else {
37-
return [collection, newItem];
44+
const b = [collection, newItem] as BrandedArray<T>;
45+
b[branded] = true;
46+
return b;
3847
}
3948
}
4049

4150
function iterate<T extends object>(collection: OneOrMany<T>, fn: (item: T) => void) {
42-
if (Array.isArray(collection)) {
51+
if (isBrandedArray(collection)) {
4352
collection.forEach(fn);
4453
} else if (collection !== null) {
4554
fn(collection);
@@ -49,14 +58,14 @@ function iterate<T extends object>(collection: OneOrMany<T>, fn: (item: T) => vo
4958
function remove<T extends object>(collection: OneOrMany<T>, item: T, message: string | false) {
5059
if (DEBUG) {
5160
let collectionIsItem = collection === item;
52-
let collectionContainsItem = Array.isArray(collection) && collection.indexOf(item) !== -1;
61+
let collectionContainsItem = isBrandedArray(collection) && collection.indexOf(item) !== -1;
5362

5463
if (!collectionIsItem && !collectionContainsItem) {
5564
throw new Error(String(message));
5665
}
5766
}
5867

59-
if (Array.isArray(collection) && collection.length > 1) {
68+
if (isBrandedArray(collection) && collection.length > 1) {
6069
let index = collection.indexOf(item);
6170
collection.splice(index, 1);
6271
return collection;

packages/@glimmer/destroyable/test/destroyables-test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,16 @@ module('Destroyables', (hooks) => {
235235
assert.verifySteps(['parent'], 'parent destructor run');
236236
});
237237

238+
test('parent can be an array', (assert) => {
239+
const parent: unknown[] = ['a'];
240+
const child = {};
241+
associateDestroyableChild(parent, child);
242+
registerDestructor(child, () => assert.step('child'));
243+
destroy(child);
244+
flush();
245+
assert.verifySteps(['child'], 'child destructor run');
246+
});
247+
238248
test('children can have multiple parents, but only destroy once', (assert) => {
239249
const parent1 = {};
240250
const parent2 = {};

0 commit comments

Comments
 (0)