From 4e8e689572dcae0cb468989c5e0c531a837a900b Mon Sep 17 00:00:00 2001 From: Evan You Date: Wed, 1 Jul 2020 19:50:04 -0400 Subject: [PATCH] fix: ensure vnode hooks are called consistently regardless of keep-alive --- .../__tests__/components/KeepAlive.spec.ts | 12 ++-- .../runtime-core/src/components/KeepAlive.ts | 59 ++++++++++++++----- packages/runtime-core/src/renderer.ts | 25 ++++---- 3 files changed, 63 insertions(+), 33 deletions(-) diff --git a/packages/runtime-core/__tests__/components/KeepAlive.spec.ts b/packages/runtime-core/__tests__/components/KeepAlive.spec.ts index bf901f6b1e6..3a3d85eeac2 100644 --- a/packages/runtime-core/__tests__/components/KeepAlive.spec.ts +++ b/packages/runtime-core/__tests__/components/KeepAlive.spec.ts @@ -567,7 +567,7 @@ describe('KeepAlive', () => { }) }) - it('should not call onVnodeUnmounted', async () => { + it('should call correct vnode hooks', async () => { const Foo = markRaw({ name: 'Foo', render() { @@ -643,14 +643,16 @@ describe('KeepAlive', () => { await nextTick() expect(spyMounted).toHaveBeenCalledTimes(2) - expect(spyUnmounted).toHaveBeenCalledTimes(0) + expect(spyUnmounted).toHaveBeenCalledTimes(1) toggle() await nextTick() + expect(spyMounted).toHaveBeenCalledTimes(3) + expect(spyUnmounted).toHaveBeenCalledTimes(2) + render(null, root) await nextTick() - - expect(spyMounted).toHaveBeenCalledTimes(2) - expect(spyUnmounted).toHaveBeenCalledTimes(2) + expect(spyMounted).toHaveBeenCalledTimes(3) + expect(spyUnmounted).toHaveBeenCalledTimes(4) }) }) diff --git a/packages/runtime-core/src/components/KeepAlive.ts b/packages/runtime-core/src/components/KeepAlive.ts index 4e18ec442b7..4cc7747b626 100644 --- a/packages/runtime-core/src/components/KeepAlive.ts +++ b/packages/runtime-core/src/components/KeepAlive.ts @@ -23,7 +23,8 @@ import { queuePostRenderEffect, MoveType, RendererElement, - RendererNode + RendererNode, + invokeVNodeHook } from '../renderer' import { setTransitionHooks } from './BaseTransition' import { ComponentRenderContext } from '../componentProxy' @@ -96,11 +97,11 @@ const KeepAliveImpl = { const storageContainer = createElement('div') sharedContext.activate = (vnode, container, anchor, isSVG, optimized) => { - const child = vnode.component! + const instance = vnode.component! move(vnode, container, anchor, MoveType.ENTER, parentSuspense) // in case props have changed patch( - child.vnode, + instance.vnode, vnode, container, anchor, @@ -110,27 +111,35 @@ const KeepAliveImpl = { optimized ) queuePostRenderEffect(() => { - child.isDeactivated = false - if (child.a) { - invokeArrayFns(child.a) + instance.isDeactivated = false + if (instance.a) { + invokeArrayFns(instance.a) + } + const vnodeHook = vnode.props && vnode.props.onVnodeMounted + if (vnodeHook) { + invokeVNodeHook(vnodeHook, instance.parent, vnode) } }, parentSuspense) } sharedContext.deactivate = (vnode: VNode) => { + const instance = vnode.component! move(vnode, storageContainer, null, MoveType.LEAVE, parentSuspense) queuePostRenderEffect(() => { - const component = vnode.component! - if (component.da) { - invokeArrayFns(component.da) + if (instance.da) { + invokeArrayFns(instance.da) + } + const vnodeHook = vnode.props && vnode.props.onVnodeUnmounted + if (vnodeHook) { + invokeVNodeHook(vnodeHook, instance.parent, vnode) } - component.isDeactivated = true + instance.isDeactivated = true }, parentSuspense) } function unmount(vnode: VNode) { // reset the shapeFlag so it can be properly unmounted - vnode.shapeFlag = ShapeFlags.STATEFUL_COMPONENT + resetShapeFlag(vnode) _unmount(vnode, instance, parentSuspense) } @@ -150,7 +159,7 @@ const KeepAliveImpl = { } else if (current) { // current active instance should no longer be kept-alive. // we can't unmount it now but it might be later, so reset its flag now. - current.shapeFlag = ShapeFlags.STATEFUL_COMPONENT + resetShapeFlag(current) } cache.delete(key) keys.delete(key) @@ -165,7 +174,18 @@ const KeepAliveImpl = { ) onBeforeUnmount(() => { - cache.forEach(unmount) + cache.forEach(cached => { + const { subTree, suspense } = instance + if (cached.type === subTree.type) { + // current instance will be unmounted as part of keep-alive's unmount + resetShapeFlag(subTree) + // but invoke its deactivated hook here + const da = subTree.component!.da + da && queuePostRenderEffect(da, suspense) + return + } + unmount(cached) + }) }) return () => { @@ -197,7 +217,7 @@ const KeepAliveImpl = { (include && (!name || !matches(include, name))) || (exclude && name && matches(exclude, name)) ) { - return vnode + return (current = vnode) } const key = vnode.key == null ? comp : vnode.key @@ -325,3 +345,14 @@ function injectToKeepAliveRoot( remove(keepAliveRoot[type]!, hook) }, target) } + +function resetShapeFlag(vnode: VNode) { + let shapeFlag = vnode.shapeFlag + if (shapeFlag & ShapeFlags.COMPONENT_SHOULD_KEEP_ALIVE) { + shapeFlag -= ShapeFlags.COMPONENT_SHOULD_KEEP_ALIVE + } + if (shapeFlag & ShapeFlags.COMPONENT_KEPT_ALIVE) { + shapeFlag -= ShapeFlags.COMPONENT_KEPT_ALIVE + } + vnode.shapeFlag = shapeFlag +} diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts index e81cc87a63b..de69659b050 100644 --- a/packages/runtime-core/src/renderer.ts +++ b/packages/runtime-core/src/renderer.ts @@ -1866,25 +1866,25 @@ function baseCreateRenderer( patchFlag, 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) && !shouldKeepAlive) { + if (shapeFlag & ShapeFlags.COMPONENT_SHOULD_KEEP_ALIVE) { + ;(parentComponent!.ctx as KeepAliveContext).deactivate(vnode) + return + } + + const shouldInvokeDirs = shapeFlag & ShapeFlags.ELEMENT && dirs + + let vnodeHook: VNodeHook | undefined | null + if ((vnodeHook = props && props.onVnodeBeforeUnmount)) { invokeVNodeHook(vnodeHook, parentComponent, vnode) } if (shapeFlag & ShapeFlags.COMPONENT) { - if (shouldKeepAlive) { - ;(parentComponent!.ctx as KeepAliveContext).deactivate(vnode) - } else { - unmountComponent(vnode.component!, parentSuspense, doRemove) - } + unmountComponent(vnode.component!, parentSuspense, doRemove) } else { if (__FEATURE_SUSPENSE__ && shapeFlag & ShapeFlags.SUSPENSE) { vnode.suspense!.unmount(parentSuspense, doRemove) @@ -1917,10 +1917,7 @@ function baseCreateRenderer( } } - if ( - ((vnodeHook = props && props.onVnodeUnmounted) || shouldInvokeDirs) && - !shouldKeepAlive - ) { + if ((vnodeHook = props && props.onVnodeUnmounted) || shouldInvokeDirs) { queuePostRenderEffect(() => { vnodeHook && invokeVNodeHook(vnodeHook, parentComponent, vnode) shouldInvokeDirs &&