Skip to content

Commit

Permalink
fix: ensure vnode hooks are called consistently regardless of keep-alive
Browse files Browse the repository at this point in the history
  • Loading branch information
yyx990803 committed Jul 1, 2020
1 parent c9629f2 commit 4e8e689
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 33 deletions.
12 changes: 7 additions & 5 deletions packages/runtime-core/__tests__/components/KeepAlive.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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)
})
})
59 changes: 45 additions & 14 deletions packages/runtime-core/src/components/KeepAlive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import {
queuePostRenderEffect,
MoveType,
RendererElement,
RendererNode
RendererNode,
invokeVNodeHook
} from '../renderer'
import { setTransitionHooks } from './BaseTransition'
import { ComponentRenderContext } from '../componentProxy'
Expand Down Expand Up @@ -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,
Expand All @@ -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)
}

Expand All @@ -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)
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
25 changes: 11 additions & 14 deletions packages/runtime-core/src/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 &&
Expand Down

0 comments on commit 4e8e689

Please sign in to comment.