Skip to content

Commit

Permalink
Manually clean up <Portal> on destroy
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rgossiaux committed Feb 27, 2022
1 parent 87aadf2 commit b18fd20
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 0 deletions.
43 changes: 43 additions & 0 deletions src/lib/components/portal/portal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
5 changes: 5 additions & 0 deletions src/lib/hooks/use-portal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

1 comment on commit b18fd20

@vercel
Copy link

@vercel vercel bot commented on b18fd20 Feb 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.