From 2fe4b53b364bd5c0f52ccd658f4b09b3a5f2ae1f Mon Sep 17 00:00:00 2001
From: Thomas Marcucci <thomas.marcucci@shopify.com>
Date: Thu, 2 Jan 2025 14:51:24 -0500
Subject: [PATCH] Prevents removeChild from removing any nodes when child is
 not found

---
 .changeset/red-waves-flash.md            |  5 +++
 packages/core/src/receiver.ts            |  4 +-
 packages/core/src/root.ts                | 12 ++---
 packages/core/src/tests/receiver.test.ts | 56 +++++++++++++++++++++++-
 packages/core/src/tests/root.test.ts     | 15 +++++++
 5 files changed, 84 insertions(+), 8 deletions(-)
 create mode 100644 .changeset/red-waves-flash.md

diff --git a/.changeset/red-waves-flash.md b/.changeset/red-waves-flash.md
new file mode 100644
index 00000000..d68b64f0
--- /dev/null
+++ b/.changeset/red-waves-flash.md
@@ -0,0 +1,5 @@
+---
+'@remote-ui/core': patch
+---
+
+Fixes an issue when removeChild does not find a child node
diff --git a/packages/core/src/receiver.ts b/packages/core/src/receiver.ts
index 20970699..c2e58d39 100644
--- a/packages/core/src/receiver.ts
+++ b/packages/core/src/receiver.ts
@@ -202,8 +202,10 @@ export function createRemoteReceiver(): RemoteReceiver {
       ) as RemoteReceiverAttachableRoot;
 
       const {children} = attached;
-
       const [removed] = children.splice(index, 1);
+      if (!removed) {
+        return;
+      }
       attached.version += 1;
 
       detach(removed);
diff --git a/packages/core/src/root.ts b/packages/core/src/root.ts
index 41a0bf47..9002e43f 100644
--- a/packages/core/src/root.ts
+++ b/packages/core/src/root.ts
@@ -841,16 +841,16 @@ function removeChild(
 ) {
   const {strict} = rootInternals;
 
+  const childIndex = container.children.indexOf(child as any);
+  if (childIndex === -1) {
+    return undefined;
+  }
+
   return perform(container, rootInternals, {
     remote: (channel) =>
-      channel(
-        ACTION_REMOVE_CHILD,
-        (container as any).id,
-        container.children.indexOf(child as any),
-      ),
+      channel(ACTION_REMOVE_CHILD, (container as any).id, childIndex),
     local: () => {
       removeNodeFromContainer(child, rootInternals);
-
       const newChildren = [...internals.children];
       newChildren.splice(newChildren.indexOf(child), 1);
       internals.children = strict ? Object.freeze(newChildren) : newChildren;
diff --git a/packages/core/src/tests/receiver.test.ts b/packages/core/src/tests/receiver.test.ts
index 7d87ae6b..491389e4 100644
--- a/packages/core/src/tests/receiver.test.ts
+++ b/packages/core/src/tests/receiver.test.ts
@@ -1,5 +1,10 @@
 import {createRemoteReceiver} from '../receiver';
-import {ACTION_MOUNT} from '../types';
+import {
+  ACTION_MOUNT,
+  ACTION_INSERT_CHILD,
+  ACTION_REMOVE_CHILD,
+  KIND_COMPONENT,
+} from '../types';
 
 describe('createRemoteReceiver()', () => {
   describe('state', () => {
@@ -14,4 +19,53 @@ describe('createRemoteReceiver()', () => {
       expect(receiver).toHaveProperty('state', 'mounted');
     });
   });
+
+  describe('removeChild', () => {
+    it('removes a given child node', () => {
+      const receiver = createRemoteReceiver();
+      receiver.receive(ACTION_MOUNT, [
+        {
+          id: 'child1',
+          children: [],
+          kind: KIND_COMPONENT,
+          props: {},
+          type: '',
+        },
+      ]);
+
+      expect(receiver.attached.root.children).toHaveLength(1);
+      expect(receiver.attached.root.children[0].id).toBe('child1');
+
+      receiver.receive(
+        ACTION_INSERT_CHILD,
+        undefined,
+        0,
+        {
+          id: 'child2',
+          children: [],
+          kind: KIND_COMPONENT,
+          props: {},
+          type: '',
+        },
+        'child2',
+      );
+
+      expect(receiver.attached.root.children).toHaveLength(2);
+      expect(receiver.attached.root.children[0].id).toBe('child2');
+      expect(receiver.attached.root.children[1].id).toBe('child1');
+
+      receiver.receive(ACTION_REMOVE_CHILD, undefined, 0);
+
+      expect(receiver.attached.root.children).toHaveLength(1);
+      expect(receiver.attached.root.children[0].id).toBe('child1');
+    });
+
+    it('handles a missing child', () => {
+      const receiver = createRemoteReceiver();
+      receiver.receive(ACTION_MOUNT, []);
+      receiver.receive(ACTION_REMOVE_CHILD, undefined, -1);
+
+      expect(receiver.attached.root.children).toHaveLength(0);
+    });
+  });
 });
diff --git a/packages/core/src/tests/root.test.ts b/packages/core/src/tests/root.test.ts
index 101efa06..a40a343f 100644
--- a/packages/core/src/tests/root.test.ts
+++ b/packages/core/src/tests/root.test.ts
@@ -125,6 +125,21 @@ describe('root', () => {
     });
   });
 
+  describe('removeChild', () => {
+    it('does not call channel when child is not found', () => {
+      const channelMock = jest.fn();
+      const root = createRemoteRoot(channelMock);
+      root.mount();
+      // clear channel call from mount
+      channelMock.mockClear();
+
+      const card = root.createComponent('Card');
+      root.removeChild(card);
+
+      expect(channelMock).not.toHaveBeenCalled();
+    });
+  });
+
   describe('hot-swapping', () => {
     it('hot-swaps function props', () => {
       const funcOne = jest.fn();