From 2dcbb401e9c626861c3c3f7c3e8b5c2ba419f774 Mon Sep 17 00:00:00 2001
From: Edward Faulkner <edward@eaf4.com>
Date: Thu, 4 Sep 2025 23:49:22 -0400
Subject: [PATCH] Bugfix: support Array as parent in associateDestroyableChild

The included test throws an assertion without the accompanying bugfix.

The issue here is that the DestroyableMeta is trying to be clever about avoiding extra allocations, so it uses the OneOrMany type a lot. But this means it cannot accurately distinguish between the case where your parent is an array vs the case where you have multiple parents.

My fix adds a Symbol to the internally-created array that represents multiple parents so it's distinguishable from a single parent that happens to be an array.
---
 packages/@glimmer/destroyable/index.ts        | 21 +++++++++++++------
 .../destroyable/test/destroyables-test.ts     | 10 +++++++++
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/packages/@glimmer/destroyable/index.ts b/packages/@glimmer/destroyable/index.ts
index 4fd0de9a1c..60afce627e 100644
--- a/packages/@glimmer/destroyable/index.ts
+++ b/packages/@glimmer/destroyable/index.ts
@@ -7,7 +7,7 @@ const DESTROYING_STATE = 1;
 const DESTROYED_STATE = 2;
 type DestroyableState = 0 | 1 | 2;
 
-type OneOrMany<T> = null | T | T[];
+type OneOrMany<T> = null | T | BrandedArray<T>;
 
 interface DestroyableMeta<T extends Destroyable> {
   source?: T;
@@ -26,19 +26,28 @@ let DESTROYABLE_META:
   | Map<Destroyable, DestroyableMeta<Destroyable>>
   | WeakMap<Destroyable, DestroyableMeta<Destroyable>> = new WeakMap();
 
+const branded = Symbol('BrandedArray');
+type BrandedArray<T> = T[] & { [branded]: true };
+
+function isBrandedArray<T>(collection: OneOrMany<T>): collection is BrandedArray<T> {
+  return Array.isArray(collection) && branded in collection;
+}
+
 function push<T extends object>(collection: OneOrMany<T>, newItem: T): OneOrMany<T> {
   if (collection === null) {
     return newItem;
-  } else if (Array.isArray(collection)) {
+  } else if (isBrandedArray(collection)) {
     collection.push(newItem);
     return collection;
   } else {
-    return [collection, newItem];
+    const b = [collection, newItem] as BrandedArray<T>;
+    b[branded] = true;
+    return b;
   }
 }
 
 function iterate<T extends object>(collection: OneOrMany<T>, fn: (item: T) => void) {
-  if (Array.isArray(collection)) {
+  if (isBrandedArray(collection)) {
     collection.forEach(fn);
   } else if (collection !== null) {
     fn(collection);
@@ -48,14 +57,14 @@ function iterate<T extends object>(collection: OneOrMany<T>, fn: (item: T) => vo
 function remove<T extends object>(collection: OneOrMany<T>, item: T, message: string | false) {
   if (import.meta.env.DEV) {
     let collectionIsItem = collection === item;
-    let collectionContainsItem = Array.isArray(collection) && collection.indexOf(item) !== -1;
+    let collectionContainsItem = isBrandedArray(collection) && collection.indexOf(item) !== -1;
 
     if (!collectionIsItem && !collectionContainsItem) {
       throw new Error(String(message));
     }
   }
 
-  if (Array.isArray(collection) && collection.length > 1) {
+  if (isBrandedArray(collection) && collection.length > 1) {
     let index = collection.indexOf(item);
     collection.splice(index, 1);
     return collection;
diff --git a/packages/@glimmer/destroyable/test/destroyables-test.ts b/packages/@glimmer/destroyable/test/destroyables-test.ts
index d1b56864a7..155d89062e 100644
--- a/packages/@glimmer/destroyable/test/destroyables-test.ts
+++ b/packages/@glimmer/destroyable/test/destroyables-test.ts
@@ -251,6 +251,16 @@ module('Destroyables', (hooks) => {
     assert.verifySteps(['parent'], 'parent destructor run');
   });
 
+  test('parent can be an array', (assert) => {
+    const parent: unknown[] = ['a'];
+    const child = {};
+    associateDestroyableChild(parent, child);
+    registerDestructor(child, () => assert.step('child'));
+    destroy(child);
+    flush();
+    assert.verifySteps(['child'], 'child destructor run');
+  });
+
   test('children can have multiple parents, but only destroy once', (assert) => {
     const parent1 = {};
     const parent2 = {};