wip(vapor): fix component unmount when not at block root level

packages/runtime-vapor/__tests__/apiLifecycle.spec.ts

@@ -21,8 +21,10 @@ import {
 } from '@vue/runtime-dom'
 import {
   createComponent,
+  createFor,
   createIf,
   createTextNode,
+  insert,
   renderEffect,
   setText,
   template,
@@ -526,4 +528,90 @@ describe('api: lifecycle hooks', () => {
     expect(handleUpdated).toHaveBeenCalledTimes(1)
     expect(handleUpdatedChild).toHaveBeenCalledTimes(1)
   })
+
+  test('unmount hooks when nested in if block', async () => {
+    const toggle = ref(true)
+    const fn = vi.fn(() => {
+      expect(host.innerHTML).toBe('<div><span></span></div><!--if-->')
+    })
+    const fn2 = vi.fn(() => {
+      expect(host.innerHTML).toBe('<!--if-->')
+    })
+    const { render, host } = define({
+      setup() {
+        const n0 = createIf(
+          () => toggle.value,
+          () => {
+            const n1 = document.createElement('div')
+            const n2 = createComponent(Child)
+            insert(n2, n1)
+            return n1
+          },
+        )
+        return n0
+      },
+    })
+
+    const Child = {
+      setup() {
+        onBeforeUnmount(fn)
+        onUnmounted(fn2)
+
+        const t0 = template('<span></span>')
+        const n0 = t0()
+        return n0
+      },
+    }
+
+    render()
+
+    toggle.value = false
+    await nextTick()
+    expect(fn).toHaveBeenCalledTimes(1)
+    expect(fn2).toHaveBeenCalledTimes(1)
+    expect(host.innerHTML).toBe('<!--if-->')
+  })
+
+  test('unmount hooks when nested in for blocks', async () => {
+    const list = ref([1])
+    const fn = vi.fn(() => {
+      expect(host.innerHTML).toBe('<div><span></span></div><!--for-->')
+    })
+    const fn2 = vi.fn(() => {
+      expect(host.innerHTML).toBe('<!--for-->')
+    })
+    const { render, host } = define({
+      setup() {
+        const n0 = createFor(
+          () => list.value,
+          () => {
+            const n1 = document.createElement('div')
+            const n2 = createComponent(Child)
+            insert(n2, n1)
+            return n1
+          },
+        )
+        return n0
+      },
+    })
+
+    const Child = {
+      setup() {
+        onBeforeUnmount(fn)
+        onUnmounted(fn2)
+
+        const t0 = template('<span></span>')
+        const n0 = t0()
+        return n0
+      },
+    }
+
+    render()
+
+    list.value.pop()
+    await nextTick()
+    expect(fn).toHaveBeenCalledTimes(1)
+    expect(fn2).toHaveBeenCalledTimes(1)
+    expect(host.innerHTML).toBe('<!--for-->')
+  })
 })

packages/runtime-vapor/src/apiCreateFor.ts

@@ -75,7 +75,7 @@ export const createFor = (
   let newBlocks: ForBlock[]
   let parent: ParentNode | undefined | null
   const parentAnchor = __DEV__ ? createComment('for') : createTextNode()
-  const ref = new VaporFragment(oldBlocks)
+  const frag = new VaporFragment(oldBlocks)
   const instance = currentInstance!
 
   if (__DEV__ && !instance) {
@@ -265,9 +265,9 @@ export const createFor = (
       }
     }
 
-    ref.nodes = [(oldBlocks = newBlocks)]
+    frag.nodes = [(oldBlocks = newBlocks)]
     if (parentAnchor) {
-      ref.nodes.push(parentAnchor)
+      frag.nodes.push(parentAnchor)
     }
   }
 
@@ -338,12 +338,12 @@ export const createFor = (
   }
 
   const unmount = ({ nodes, scope }: ForBlock) => {
-    removeBlock(nodes, parent!)
     scope && scope.stop()
+    removeBlock(nodes, parent!)
   }
 
   once ? renderList() : renderEffect(renderList)
-  return ref
+  return frag
 }
 
 export function createForSlots(

packages/runtime-vapor/src/block.ts

@@ -21,7 +21,7 @@ export class VaporFragment {
   nodes: Block
   anchor?: Node
   insert?: (parent: ParentNode, anchor: Node | null) => void
-  remove?: () => void
+  remove?: (parent?: ParentNode) => void
 
   constructor(nodes: Block) {
     this.nodes = nodes
@@ -139,16 +139,12 @@ export function prepend(parent: ParentNode, ...blocks: Block[]): void {
  * during each root remove call, and update their children list by filtering
  * unmounted children
  */
-export let parentsWithUnmountedChildren: Set<VaporComponentInstance> | null =
-  null
+// export let parentsWithUnmountedChildren: Set<VaporComponentInstance> | null =
+//   null
 
-export function remove(block: Block, parent: ParentNode): void {
-  const isRoot = !parentsWithUnmountedChildren
-  if (isRoot) {
-    parentsWithUnmountedChildren = new Set()
-  }
+export function remove(block: Block, parent?: ParentNode): void {
   if (block instanceof Node) {
-    parent.removeChild(block)
+    parent && parent.removeChild(block)
   } else if (isVaporComponent(block)) {
     unmountComponent(block, parent)
   } else if (isArray(block)) {
@@ -158,7 +154,7 @@ export function remove(block: Block, parent: ParentNode): void {
   } else {
     // fragment
     if (block.remove) {
-      block.remove()
+      block.remove(parent)
     } else {
       remove(block.nodes, parent)
     }
@@ -167,12 +163,6 @@ export function remove(block: Block, parent: ParentNode): void {
       ;(block as DynamicFragment).scope!.stop()
     }
   }
-  if (isRoot) {
-    for (const i of parentsWithUnmountedChildren!) {
-      i.children = i.children.filter(n => !n.isUnmounted)
-    }
-    parentsWithUnmountedChildren = null
-  }
 }
 
 /**

packages/runtime-vapor/src/component.ts

@@ -1,5 +1,4 @@
 import {
-  type ComponentInternalInstance,
   type ComponentInternalOptions,
   type ComponentPropsOptions,
   EffectScope,
@@ -26,29 +25,17 @@ import {
   unregisterHMR,
   warn,
 } from '@vue/runtime-dom'
-import {
-  type Block,
-  insert,
-  isBlock,
-  parentsWithUnmountedChildren,
-  remove,
-} from './block'
+import { type Block, insert, isBlock, remove } from './block'
 import {
   type ShallowRef,
   markRaw,
+  onScopeDispose,
   pauseTracking,
   proxyRefs,
   resetTracking,
   unref,
 } from '@vue/reactivity'
-import {
-  EMPTY_ARR,
-  EMPTY_OBJ,
-  invokeArrayFns,
-  isFunction,
-  isString,
-  remove as removeItem,
-} from '@vue/shared'
+import { EMPTY_OBJ, invokeArrayFns, isFunction, isString } from '@vue/shared'
 import {
   type DynamicPropsSource,
   type RawProps,
@@ -264,6 +251,8 @@ export function createComponent(
     endMeasure(instance, 'init')
   }
 
+  onScopeDispose(() => unmountComponent(instance), true)
+
   return instance
 }
 
@@ -300,8 +289,6 @@ export class VaporComponentInstance implements GenericComponentInstance {
   type: VaporComponent
   root: GenericComponentInstance | null
   parent: GenericComponentInstance | null
-  children: VaporComponentInstance[]
-  vdomChildren?: ComponentInternalInstance[]
   appContext: GenericAppContext
 
   block: Block
@@ -379,12 +366,8 @@ export class VaporComponentInstance implements GenericComponentInstance {
     this.type = comp
     this.parent = currentInstance
     this.root = currentInstance ? currentInstance.root : this
-    this.children = []
 
     if (currentInstance) {
-      if (isVaporComponent(currentInstance)) {
-        currentInstance.children.push(this)
-      }
       this.appContext = currentInstance.appContext
       this.provides = currentInstance.provides
       this.ids = currentInstance.ids
@@ -523,40 +506,13 @@ export function unmountComponent(
 
     instance.scope.stop()
 
-    for (const c of instance.children) {
-      unmountComponent(c)
-    }
-    instance.children = EMPTY_ARR as any
-
-    if (instance.vdomChildren) {
-      const unmount = instance.appContext.vapor!.vdomUnmount
-      for (const c of instance.vdomChildren) {
-        unmount(c, null)
-      }
-      instance.vdomChildren = EMPTY_ARR as any
-    }
-
-    if (parentNode) {
-      // root remove: need to both remove this instance's DOM nodes
-      // and also remove it from the parent's children list.
-      remove(instance.block, parentNode)
-      const parentInstance = instance.parent
-      instance.parent = null
-      if (isVaporComponent(parentInstance)) {
-        if (parentsWithUnmountedChildren) {
-          // for optimize children removal
-          parentsWithUnmountedChildren.add(parentInstance)
-        } else {
-          removeItem(parentInstance.children, instance)
-        }
-      }
-    }
-
     if (instance.um) {
       queuePostFlushCb(() => invokeArrayFns(instance.um!))
     }
     instance.isUnmounted = true
-  } else if (parentNode) {
+  }
+
+  if (parentNode) {
     remove(instance.block, parentNode)
   }
 }

packages/runtime-vapor/src/vdomInterop.ts

@@ -24,7 +24,7 @@ import {
   unmountComponent,
 } from './component'
 import { type Block, VaporFragment, insert, remove } from './block'
-import { extend, isFunction, remove as removeItem } from '@vue/shared'
+import { extend, isFunction } from '@vue/shared'
 import { type RawProps, rawPropsProxyHandlers } from './componentProps'
 import type { RawSlots, VaporSlot } from './componentSlots'
 import { renderEffect } from './renderEffect'
@@ -116,17 +116,14 @@ function createVDOMComponent(
         undefined,
         false,
       )
-      ;(parentInstance.vdomChildren || (parentInstance.vdomChildren = [])).push(
-        vnode.component!,
-      )
+      // TODO register unmount with onScopeDispose
       isMounted = true
     } else {
       // TODO move
     }
   }
-  frag.remove = () => {
-    internals.umt(vnode.component!, null, true)
-    removeItem(parentInstance.vdomChildren!, vnode.component)
+  frag.remove = parentNode => {
+    internals.umt(vnode.component!, null, !!parentNode)
   }
 
   return frag
@@ -144,11 +141,9 @@ function renderVDOMSlot(
 
   let isMounted = false
   let fallbackNodes: Block | undefined
-  let parentNode: ParentNode
   let oldVNode: VNode | null = null
 
-  frag.insert = (parent, anchor) => {
-    parentNode = parent
+  frag.insert = (parentNode, anchor) => {
     if (!isMounted) {
       renderEffect(() => {
         const vnode = renderSlot(
@@ -169,7 +164,7 @@ function renderVDOMSlot(
             if (oldVNode) {
               internals.um(oldVNode, parentComponent as any, null, true)
             }
-            insert((fallbackNodes = fallback(props)), parent, anchor)
+            insert((fallbackNodes = fallback(props)), parentNode, anchor)
           }
           oldVNode = null
         }
@@ -179,7 +174,7 @@ function renderVDOMSlot(
       // TODO move
     }
 
-    frag.remove = () => {
+    frag.remove = parentNode => {
       if (fallbackNodes) {
         remove(fallbackNodes, parentNode)
       } else if (oldVNode) {