Don’t unmount portal targets used by other portals (#3144)

* Refactor

* Don’t unmount portal targets used by other portals

* Add test

* Update changelog

Author: thecrypticace

packages/@headlessui-vue/CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Prevent closing the `Combobox` component when clicking inside the scrollbar area ([#3104](https://github.com/tailwindlabs/headlessui/pull/3104))
+- Don’t unmount portal targets used by other portals ([#3144](https://github.com/tailwindlabs/headlessui/pull/3144))
 
 ## [1.7.20] - 2024-04-15
 

packages/@headlessui-vue/src/components/portal/portal.test.ts

@@ -391,3 +391,48 @@ it('should be possible to force the Portal into a specific element using PortalG
     `"<div><div><div data-v-app=\\"\\"><main><aside id=\\"group-1\\">A<div data-headlessui-portal=\\"\\">Next to A</div></aside><section id=\\"group-2\\"><span>B</span></section><!--teleport start--><!--teleport end--></main></div></div></div>"`
   )
 })
+
+it('the root shared by multiple portals should not unmount when they change in the same tick', async () => {
+  let a = ref(false)
+  let b = ref(false)
+
+  renderTemplate({
+    template: html`
+      <main>
+        <Portal v-if="a">Portal A</Portal>
+        <Portal v-if="b">Portal B</Portal>
+      </main>
+    `,
+    setup: () => ({ a, b }),
+  })
+
+  await new Promise<void>(nextTick)
+
+  let root = () => document.querySelector('#headlessui-portal-root')
+
+  // There is no portal root initially because there are no visible portals
+  expect(root()).toBe(null)
+
+  // Show portal A
+  a.value = true
+  await new Promise<void>(nextTick)
+
+  // There is a portal root now because there is a visible portal
+  expect(root()).not.toBe(null)
+
+  // Swap portal A for portal B
+  a.value = false
+  b.value = true
+
+  await new Promise<void>(nextTick)
+
+  // The portal root is still there because there are still visible portals
+  expect(root()).not.toBe(null)
+
+  // Hide portal B
+  b.value = false
+  await new Promise<void>(nextTick)
+
+  // The portal root is gone because there are no visible portals
+  expect(root()).toBe(null)
+})

packages/@headlessui-vue/src/components/portal/portal.ts

@@ -44,6 +44,27 @@ function getPortalRoot(contextElement?: Element | null) {
   return ownerDocument.body.appendChild(root)
 }
 
+// A counter that keeps track of how many portals are currently using
+// a specific portal root. This is used to know when we can safely
+// remove the portal root from the DOM.
+const counter = new WeakMap<HTMLElement, number>()
+
+function getCount(el: HTMLElement): number {
+  return counter.get(el) ?? 0
+}
+
+function setCount(el: HTMLElement, cb: (val: number) => number): number {
+  let newCount = cb(getCount(el))
+
+  if (newCount <= 0) {
+    counter.delete(el)
+  } else {
+    counter.set(el, newCount)
+  }
+
+  return newCount
+}
+
 export let Portal = defineComponent({
   name: 'Portal',
   props: {
@@ -63,6 +84,11 @@ export let Portal = defineComponent({
         : groupContext.resolveTarget()
     )
 
+    // Make a note that we are using this element
+    if (myTarget.value) {
+      setCount(myTarget.value, (val) => val + 1)
+    }
+
     let ready = ref(false)
     onMounted(() => {
       ready.value = true
@@ -95,9 +121,17 @@ export let Portal = defineComponent({
       if (!root) return
       if (myTarget.value !== root) return
 
-      if (myTarget.value.children.length <= 0) {
-        myTarget.value.parentElement?.removeChild(myTarget.value)
-      }
+      // We no longer need the portal target
+      let remaining = setCount(myTarget.value, (val) => val - 1)
+
+      // However, if another portal is still using the same target
+      // we should not remove it.
+      if (remaining) return
+
+      // There are still children in the portal, we should not remove it.
+      if (myTarget.value.children.length > 0) return
+
+      myTarget.value.parentElement?.removeChild(myTarget.value)
     })
 
     return () => {