From b18fd2093b81ee72cdd32173644d0fe43293fab3 Mon Sep 17 00:00:00 2001
From: Ryan Gossiaux <ryan.gossiaux@gmail.com>
Date: Sun, 27 Feb 2022 15:18:36 -0800
Subject: [PATCH] Manually clean up <Portal> on destroy

Fixes #68

In some cases, when Svelte sees a parent and a child, it will detach the parent and assume that this suffices to detach the child. However, the <Portal> moves elements around in the DOM, so this assumption does not always hold. We need to make sure we detach the portal element ourselves.
---
 src/lib/components/portal/portal.test.ts | 43 ++++++++++++++++++++++++
 src/lib/hooks/use-portal.ts              |  5 +++
 2 files changed, 48 insertions(+)

diff --git a/src/lib/components/portal/portal.test.ts b/src/lib/components/portal/portal.test.ts
index 580d07a..9dc518a 100644
--- a/src/lib/components/portal/portal.test.ts
+++ b/src/lib/components/portal/portal.test.ts
@@ -290,3 +290,46 @@ it('should be possible to force the Portal into a specific element using PortalG
     `"<div><main><aside id=\\"group-1\\">A<div>Next to A</div></aside> <section id=\\"group-2\\"><span>B</span></section>  </main></div><div id=\\"headlessui-portal-root\\"><div>I am in the portal root</div></div>"`
   )
 })
+
+it('should cleanup the Portal properly when Svelte would not detach it', async () => {
+  expect(getPortalRoot()).toBe(null)
+
+  render(svelte`
+    <script>
+      let render = false;
+    </script>
+    <main id="parent">
+      <button id="a" on:click={() => render = !render}>
+        Toggle
+      </button>
+      {#if render}
+        <div>
+          <Portal>
+            <p id="content1">Contents 1 ...</p>
+          </Portal>
+        </div>
+      {/if}
+    </main>
+    `)
+
+  let a = document.getElementById('a')
+
+  expect(getPortalRoot()).toBe(null)
+
+  // Let's render the first Portal
+  await click(a)
+
+  expect(getPortalRoot()).not.toBe(null)
+  expect(getPortalRoot().childNodes).toHaveLength(1)
+
+  // Let's remove the first portal
+  await click(a)
+
+  expect(getPortalRoot()).toBe(null)
+
+  // Let's render the first Portal again
+  await click(a)
+
+  expect(getPortalRoot()).not.toBe(null)
+  expect(getPortalRoot().childNodes).toHaveLength(1)
+})
diff --git a/src/lib/hooks/use-portal.ts b/src/lib/hooks/use-portal.ts
index ebc32aa..bbda7e5 100644
--- a/src/lib/hooks/use-portal.ts
+++ b/src/lib/hooks/use-portal.ts
@@ -11,6 +11,11 @@ export function portal(
       newTarget.append(element);
     },
     destroy() {
+      // Need to detach ourselves--we can't rely on Svelte always detaching
+      // us since we moved in the component tree.
+      if (target?.contains(element)) {
+        target.removeChild(element);
+      }
       if (target && target.childNodes.length <= 0) {
         target.parentElement?.removeChild(target);
       }