fix(keep-alive): do not invoke onVnodeBeforeUnmount if is KeepAlive component (#1079)

Author: pikax

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

@@ -7,7 +7,14 @@ import {
   KeepAlive,
   serializeInner,
   nextTick,
-  ComponentOptions
+  ComponentOptions,
+  markRaw,
+  inject,
+  defineComponent,
+  ComponentPublicInstance,
+  Ref,
+  cloneVNode,
+  provide
 } from '@vue/runtime-test'
 import { KeepAliveProps } from '../../src/components/KeepAlive'
 
@@ -559,4 +566,91 @@ describe('KeepAlive', () => {
       expect(serializeInner(root)).toBe(`1`)
     })
   })
+
+  it('should not call onVnodeUnmounted', async () => {
+    const Foo = markRaw({
+      name: 'Foo',
+      render() {
+        return h('Foo')
+      }
+    })
+    const Bar = markRaw({
+      name: 'Bar',
+      render() {
+        return h('Bar')
+      }
+    })
+
+    const spyMounted = jest.fn()
+    const spyUnmounted = jest.fn()
+
+    const RouterView = defineComponent({
+      setup(_, { slots }) {
+        const Component = inject<Ref<ComponentPublicInstance>>('component')
+        const refView = ref()
+
+        let componentProps = {
+          ref: refView,
+          onVnodeMounted() {
+            spyMounted()
+          },
+          onVnodeUnmounted() {
+            spyUnmounted()
+          }
+        }
+
+        return () => {
+          const child: any = slots.default!({
+            Component: Component!.value
+          })[0]
+
+          const innerChild = child.children[0]
+          child.children[0] = cloneVNode(innerChild, componentProps)
+          return child
+        }
+      }
+    })
+
+    let toggle: () => void = () => {}
+
+    const App = defineComponent({
+      setup() {
+        const component = ref(Foo)
+
+        provide('component', component)
+
+        toggle = () => {
+          component.value = component.value === Foo ? Bar : Foo
+        }
+        return {
+          component,
+          toggle
+        }
+      },
+      render() {
+        return h(RouterView, null, {
+          default: ({ Component }: any) => h(KeepAlive, null, [h(Component)])
+        })
+      }
+    })
+
+    render(h(App), root)
+    await nextTick()
+    expect(spyMounted).toHaveBeenCalledTimes(1)
+    expect(spyUnmounted).toHaveBeenCalledTimes(0)
+
+    toggle()
+    await nextTick()
+
+    expect(spyMounted).toHaveBeenCalledTimes(2)
+    expect(spyUnmounted).toHaveBeenCalledTimes(0)
+
+    toggle()
+    await nextTick()
+    render(null, root)
+    await nextTick()
+
+    expect(spyMounted).toHaveBeenCalledTimes(2)
+    expect(spyUnmounted).toHaveBeenCalledTimes(2)
+  })
 })

packages/runtime-core/src/renderer.ts

@@ -1713,19 +1713,20 @@ function baseCreateRenderer(
   ) => {
     const { props, ref, children, dynamicChildren, shapeFlag, dirs } = vnode
     const shouldInvokeDirs = shapeFlag & ShapeFlags.ELEMENT && dirs
+    const shouldKeepAlive = shapeFlag & ShapeFlags.COMPONENT_SHOULD_KEEP_ALIVE
     let vnodeHook: VNodeHook | undefined | null
 
     // unset ref
     if (ref != null && parentComponent) {
       setRef(ref, null, parentComponent, null)
     }
 
-    if ((vnodeHook = props && props.onVnodeBeforeUnmount)) {
+    if ((vnodeHook = props && props.onVnodeBeforeUnmount) && !shouldKeepAlive) {
       invokeVNodeHook(vnodeHook, parentComponent, vnode)
     }
 
     if (shapeFlag & ShapeFlags.COMPONENT) {
-      if (shapeFlag & ShapeFlags.COMPONENT_SHOULD_KEEP_ALIVE) {
+      if (shouldKeepAlive) {
         ;(parentComponent!.ctx as KeepAliveContext).deactivate(vnode)
       } else {
         unmountComponent(vnode.component!, parentSuspense, doRemove)
@@ -1757,7 +1758,10 @@ function baseCreateRenderer(
       }
     }
 
-    if ((vnodeHook = props && props.onVnodeUnmounted) || shouldInvokeDirs) {
+    if (
+      ((vnodeHook = props && props.onVnodeUnmounted) || shouldInvokeDirs) &&
+      !shouldKeepAlive
+    ) {
       queuePostRenderEffect(() => {
         vnodeHook && invokeVNodeHook(vnodeHook, parentComponent, vnode)
         shouldInvokeDirs &&