fix(transition): handle comment-wrapped component roots (#15853)

close #6080
close #5500

Author: edison1105

packages/runtime-core/__tests__/components/BaseTransition.spec.ts

@@ -13,6 +13,8 @@ import {
   serialize,
   serializeInner,
 } from '@vue/runtime-test'
+import * as runtime from '@vue/runtime-test'
+import { compile } from '@vue/compiler-dom'
 
 function mount(
   props: BaseTransitionProps<TestElement>,
@@ -1356,4 +1358,254 @@ describe('BaseTransition', () => {
     expect(firstBeforeLeave).not.toHaveBeenCalled()
     expect(nextBeforeLeave).toHaveBeenCalledTimes(1)
   })
+
+  describe.each([true, false])('comment roots (__DEV__: %s)', dev => {
+    const roots = [
+      ['element', '<!--start--><div>A</div><!--end-->', 5],
+      ['component', '<!--start--><Leaf /><!--end-->', 5],
+      [
+        'nested fragment',
+        '<!--start--><div v-if="false">If</div><!--between--><div v-else>A</div><!--end-->',
+        dev ? 8 : 5,
+      ],
+    ] as const
+
+    test.each(
+      roots.flatMap(([name, template, nodes]) =>
+        (['out-in', 'in-out'] as const).flatMap(mode =>
+          [false, true].map(withKeepAlive => ({
+            name,
+            template,
+            nodes,
+            mode,
+            withKeepAlive,
+          })),
+        ),
+      ),
+    )('$name, $mode, KeepAlive: $withKeepAlive', async options => {
+      __DEV__ = dev
+      try {
+        const { template, nodes, mode, withKeepAlive } = options
+        const { code } = compile(template, {
+          comments: true,
+          hoistStatic: true,
+        })
+        const One = {
+          components: { Leaf: { render: () => h('div', 'A') } },
+          render: new Function('Vue', code)(runtime),
+        }
+        const Two = { render: () => h('span', 'B') }
+        const toggle = ref(true)
+        const { props, cbs } = mockProps({ mode }, withKeepAlive)
+        const { root } = mount(
+          props,
+          () => h(toggle.value ? One : Two),
+          withKeepAlive,
+        )
+        expect(root.children).toHaveLength(nodes)
+
+        for (const active of [false, true]) {
+          const leaving = active ? '<span>B</span>' : '<div>A</div>'
+          const entering = active ? '<div>A</div>' : '<span>B</span>'
+          const call = active ? 1 : 0
+          toggle.value = active
+          await nextTick()
+
+          if (mode === 'out-in') {
+            expect(props.onLeave).toHaveBeenCalledTimes(call + 1)
+            assertCalledWithEl(props.onLeave, leaving, call)
+            expect(props.onEnter).toHaveBeenCalledTimes(call)
+            cbs.doneLeave[leaving]()
+            await nextTick()
+            expect(props.onEnter).toHaveBeenCalledTimes(call + 1)
+            assertCalledWithEl(props.onEnter, entering, call)
+            cbs.doneEnter[entering]()
+          } else {
+            expect(props.onEnter).toHaveBeenCalledTimes(call + 1)
+            assertCalledWithEl(props.onEnter, entering, call)
+            expect(props.onLeave).toHaveBeenCalledTimes(call)
+            cbs.doneEnter[entering]()
+            expect(props.onLeave).toHaveBeenCalledTimes(call + 1)
+            assertCalledWithEl(props.onLeave, leaving, call)
+            cbs.doneLeave[leaving]()
+            await nextTick()
+          }
+
+          expect(props.onAfterLeave).toHaveBeenCalledTimes(call + 1)
+          expect(props.onAfterEnter).toHaveBeenCalledTimes(call + 1)
+          expect(root.children).toHaveLength(active ? nodes : 1)
+          expect(serializeInner(root)).toBe(
+            active
+              ? `<!--start-->${dev && options.name === 'nested fragment' ? '<!--between-->' : ''}<div>A</div><!--end-->`
+              : '<span>B</span>',
+          )
+        }
+      } finally {
+        __DEV__ = true
+      }
+    })
+  })
+
+  test.each([false, true])(
+    'should update leave hooks through comment roots (KeepAlive: %s)',
+    async withKeepAlive => {
+      const root = nodeOps.createElement('div')
+      const toggle = ref(true)
+      const firstLeave = vi.fn((_el, done) => done())
+      const nextLeave = vi.fn((_el, done) => done())
+      const onLeave = ref(firstLeave)
+      const { code } = compile('<!--start--><div>A</div><!--end-->')
+      const One = { render: new Function('Vue', code)(runtime) }
+      const Two = { render: () => h('span', 'B') }
+      const slot = () => h(toggle.value ? One : Two)
+      const App = () =>
+        h(BaseTransition, { onLeave: onLeave.value }, () =>
+          withKeepAlive ? h(KeepAlive, null, slot()) : slot(),
+        )
+      render(h(App), root)
+
+      onLeave.value = nextLeave
+      toggle.value = false
+      await nextTick()
+
+      expect(firstLeave).not.toHaveBeenCalled()
+      expect(nextLeave).toHaveBeenCalledTimes(1)
+      assertCalledWithEl(nextLeave, '<div>A</div>')
+      expect(root.children).toHaveLength(1)
+      expect(serializeInner(root)).toBe('<span>B</span>')
+      render(null, root)
+    },
+  )
+
+  test.each([true, false])(
+    'should skip out-in leave for empty comment roots (__DEV__: %s)',
+    async dev => {
+      __DEV__ = dev
+      try {
+        const { code } = compile(
+          '<!--start--><div v-if="false">A</div><!--end-->',
+          { comments: true },
+        )
+        const One = { render: new Function('Vue', code)(runtime) }
+        const toggle = ref(true)
+        const { props } = mockProps({ mode: 'out-in' })
+        const { root } = mount(props, () =>
+          toggle.value ? h(One) : h('span', 'B'),
+        )
+
+        toggle.value = false
+        await nextTick()
+
+        expect(props.onLeave).not.toHaveBeenCalled()
+        expect(props.onEnter).toHaveBeenCalledTimes(1)
+        expect(serializeInner(root)).toBe('<span>B</span>')
+        expect(root.children).toHaveLength(1)
+      } finally {
+        __DEV__ = true
+      }
+    },
+  )
+
+  test('should not animate a fragment with multiple element roots', async () => {
+    const { code } = compile(
+      '<!--start--><template v-if="true"><div>A</div><span>B</span></template><!--end-->',
+    )
+    const One = { render: new Function('Vue', code)(runtime) }
+    const toggle = ref(true)
+    const { props } = mockProps()
+    const { root } = mount(props, () => (toggle.value ? h(One) : h('p', 'C')))
+    expect(
+      'Component inside <Transition> renders non-element root node',
+    ).toHaveBeenWarned()
+
+    toggle.value = false
+    await nextTick()
+
+    expect(props.onLeave).not.toHaveBeenCalled()
+    expect(serializeInner(root)).toBe('<p>C</p>')
+    expect(root.children).toHaveLength(1)
+  })
+
+  test.each(['default', 'in-out'] as const)(
+    'should finish pending leave when a comment root re-enters (mode: %s)',
+    async mode => {
+      const { code } = compile('<!--start--><div>A</div><!--end-->', {
+        comments: true,
+        hoistStatic: true,
+      })
+      const One = { render: new Function('Vue', code)(runtime) }
+      const Two = { render: () => h('span', 'B') }
+      const toggle = ref(true)
+      const { props, cbs } = mockProps({ mode })
+      const { root } = mount(props, () =>
+        h(toggle.value ? One : Two, { key: toggle.value ? 'one' : 'two' }),
+      )
+      const leavingNodes = root.children.slice()
+
+      toggle.value = false
+      await nextTick()
+      cbs.doneEnter['<span>B</span>']()
+      expect(props.onLeave).toHaveBeenCalledTimes(1)
+      expect(props.onAfterLeave).not.toHaveBeenCalled()
+      const finishFirstLeave = cbs.doneLeave['<div>A</div>']
+
+      toggle.value = true
+      await nextTick()
+      expect(props.onAfterLeave).toHaveBeenCalledTimes(1)
+      assertCalledWithEl(props.onAfterLeave, '<div>A</div>')
+      expect(props.onLeaveCancelled).not.toHaveBeenCalled()
+      expect(leavingNodes.every(node => node.parentNode === null)).toBe(true)
+      expect(serializeInner(root)).toBe(
+        '<span>B</span><!--start--><div>A</div><!--end-->',
+      )
+      expect(root.children).toHaveLength(6)
+
+      cbs.doneEnter['<div>A</div>']()
+      expect(props.onLeave).toHaveBeenCalledTimes(2)
+      cbs.doneLeave['<span>B</span>']()
+      finishFirstLeave()
+      expect(props.onAfterLeave).toHaveBeenCalledTimes(2)
+      expect(serializeInner(root)).toBe('<!--start--><div>A</div><!--end-->')
+      expect(root.children).toHaveLength(5)
+    },
+  )
+
+  test.each([true, false])(
+    'should enter when a component updates its own root (__DEV__: %s)',
+    async dev => {
+      __DEV__ = dev
+      try {
+        const showA = ref(true)
+        const { code } = compile(
+          '<div v-if="showA">A</div><span v-else>B</span>',
+          { prefixIdentifiers: true },
+        )
+        const Comp = {
+          setup: () => ({ showA }),
+          render: new Function('Vue', code)(runtime),
+        }
+        const { props, cbs } = mockProps()
+        const { root } = mount(props, () => h(Comp))
+
+        showA.value = false
+        await nextTick()
+
+        expect(props.onLeave).toHaveBeenCalledTimes(1)
+        expect(props.onBeforeEnter).toHaveBeenCalledTimes(1)
+        expect(props.onEnter).toHaveBeenCalledTimes(1)
+        assertCalledWithEl(props.onEnter, '<span>B</span>')
+        expect(props.onAfterLeave).toHaveBeenCalledTimes(1)
+        assertCalledWithEl(props.onAfterLeave, '<div>A</div>')
+        expect(serializeInner(root)).toBe('<span>B</span>')
+
+        cbs.doneLeave['<div>A</div>']()
+        expect(props.onAfterLeave).toHaveBeenCalledTimes(1)
+        expect(props.onLeaveCancelled).not.toHaveBeenCalled()
+        cbs.doneEnter['<span>B</span>']()
+        expect(props.onAfterEnter).toHaveBeenCalledTimes(1)
+      } finally {
+        __DEV__ = true
+      }
+    },
+  )
 })

packages/runtime-core/src/components/BaseTransition.ts

@@ -23,6 +23,7 @@ import { isTeleport } from './Teleport'
 import type { RendererElement } from '../renderer'
 import { SchedulerJobFlags } from '../scheduler'
 import { isHmrUpdating } from '../hmr'
+import { filterSingleRoot } from '../componentRenderUtils'
 
 type Hook<T = () => void> = T | T[]
 
@@ -139,11 +140,6 @@ export const BaseTransitionPropsValidators: Record<string, any> = {
   onAppearCancelled: TransitionHookValidator,
 }
 
-const recursiveGetSubtree = (instance: ComponentInternalInstance): VNode => {
-  const subTree = instance.subTree
-  return subTree.component ? recursiveGetSubtree(subTree.component) : subTree
-}
-
 const BaseTransitionImpl: ComponentOptions = {
   name: `BaseTransition`,
 
@@ -214,7 +210,7 @@ const BaseTransitionImpl: ComponentOptions = {
         oldInnerChild &&
         oldInnerChild.type !== Comment &&
         !isSameVNodeType(oldInnerChild, innerChild) &&
-        recursiveGetSubtree(instance).type !== Comment
+        getTransitionRoot(instance.subTree)
       ) {
         let leavingHooks = resolveTransitionHooks(
           oldInnerChild,
@@ -397,13 +393,18 @@ export function resolveTransitionHooks(
       }
       // for toggled element with same key (v-if)
       const leavingVNode = leavingVNodesCache[key]
-      if (
-        leavingVNode &&
-        isSameVNodeType(vnode, leavingVNode) &&
-        (leavingVNode.el as TransitionElement)[leaveCbKey]
-      ) {
-        // force early removal (not cancelled)
-        ;(leavingVNode.el as TransitionElement)[leaveCbKey]!()
+      if (leavingVNode && isSameVNodeType(vnode, leavingVNode)) {
+        // A component's el still points to its leaving root during patch.
+        let leavingCb = (leavingVNode.el as TransitionElement)[leaveCbKey]
+        if (!leavingCb) {
+          const leavingRoot = getTransitionRoot(leavingVNode)
+          leavingCb =
+            leavingRoot && (leavingRoot.el as TransitionElement)[leaveCbKey]
+        }
+        if (leavingCb) {
+          // force early removal (not cancelled)
+          leavingCb()
+        }
       }
       callHook(hook, [el])
     },
@@ -548,6 +549,12 @@ export function setTransitionHooks(vnode: VNode, hooks: TransitionHooks): void {
     vnode.ssFallback!.transition = hooks.clone(vnode.ssFallback!)
   } else {
     vnode.transition = hooks
+    if (vnode.type === Fragment && isArray(vnode.children)) {
+      const child = filterSingleRoot(vnode.children, false)
+      if (child) {
+        setTransitionHooks(child, hooks)
+      }
+    }
   }
 }
 
@@ -588,3 +595,17 @@ export function getTransitionRawChildren(
   }
   return ret
 }
+
+export function getTransitionRoot(vnode: VNode): VNode | undefined {
+  if (vnode.component) {
+    return getTransitionRoot(vnode.component.subTree)
+  }
+  if (vnode.type === Fragment) {
+    const child =
+      isArray(vnode.children) && filterSingleRoot(vnode.children, false)
+    return child ? getTransitionRoot(child) : undefined
+  }
+  if (vnode.type !== Comment) {
+    return vnode
+  }
+}