From 9a917ba7e238d60511016d3bf6c15ab55cb7bbe6 Mon Sep 17 00:00:00 2001 From: pikax Date: Wed, 29 Apr 2020 10:44:14 +0100 Subject: [PATCH 1/3] fix(keepAlive): do not invoke onVnodeBeforeUnmount if is KeepAlive component --- .../__tests__/components/KeepAlive.spec.ts | 106 +++++++++++++++++- packages/runtime-core/src/renderer.ts | 10 +- 2 files changed, 112 insertions(+), 4 deletions(-) diff --git a/packages/runtime-core/__tests__/components/KeepAlive.spec.ts b/packages/runtime-core/__tests__/components/KeepAlive.spec.ts index be17b0772f6..b64b474b26b 100644 --- a/packages/runtime-core/__tests__/components/KeepAlive.spec.ts +++ b/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,101 @@ describe('KeepAlive', () => { expect(serializeInner(root)).toBe(`1`) }) }) + + it('should do router view', async () => { + const Foo = markRaw({ + name: 'Foo', + template: ` +
This is Foo
+ ` + }) + + const Bar = markRaw({ + name: 'Bar', + template: ` +
This is Bar
+ ` + }) + + const spyMounted = jest.fn() + const spyUnmounted = jest.fn() + + const RouterView = defineComponent({ + setup(props, { slots }) { + const Component = inject>('component') + const refView = ref() + + let componentProps = { + ref: refView, + onVnodeMounted() { + spyMounted() + console.log('mounted ref', refView.value) + }, + onVnodeUnmounted() { + spyUnmounted() + console.log('unmounted ref', refView.value) + } + } + + return () => { + const child: any = slots.default!({ + Component: Component!.value + })[0] + + const innerChild = child.children[0] as any + + child.children[0] = cloneVNode(innerChild, componentProps) + console.log('render') + + return child + } + } + }) + + let toggle: () => void = () => {} + + const App = defineComponent({ + // template: ` + // + // + // + // `, + setup() { + const component = ref(Foo) + + provide('component', component) + + toggle = () => { + component.value = component.value === Foo ? Bar : Foo + } + + return { + component, + toggle + } + // return () => { + // return h(RouterView, null, h(KeepAlive, null, h(component.value))) + // } + }, + render() { + return h(RouterView, null, { + default: ({ Component }: any) => h(KeepAlive, null, [h(Component)]) + }) + }, + components: { + RouterView + } + }) + + 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) + }) }) diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts index c31667458f9..b95488a9585 100644 --- a/packages/runtime-core/src/renderer.ts +++ b/packages/runtime-core/src/renderer.ts @@ -1705,6 +1705,7 @@ 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 @@ -1712,12 +1713,12 @@ function baseCreateRenderer( 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) @@ -1749,7 +1750,10 @@ function baseCreateRenderer( } } - if ((vnodeHook = props && props.onVnodeUnmounted) || shouldInvokeDirs) { + if ( + ((vnodeHook = props && props.onVnodeUnmounted) || shouldInvokeDirs) && + !shouldKeepAlive + ) { queuePostRenderEffect(() => { vnodeHook && invokeVNodeHook(vnodeHook, parentComponent, vnode) shouldInvokeDirs && From 17cc187d7e4585cc95c4efb505956629de94e13c Mon Sep 17 00:00:00 2001 From: pikax Date: Wed, 29 Apr 2020 11:05:49 +0100 Subject: [PATCH 2/3] test: improve test --- .../__tests__/components/KeepAlive.spec.ts | 48 ++++++++----------- 1 file changed, 20 insertions(+), 28 deletions(-) diff --git a/packages/runtime-core/__tests__/components/KeepAlive.spec.ts b/packages/runtime-core/__tests__/components/KeepAlive.spec.ts index b64b474b26b..891f8d61d44 100644 --- a/packages/runtime-core/__tests__/components/KeepAlive.spec.ts +++ b/packages/runtime-core/__tests__/components/KeepAlive.spec.ts @@ -14,7 +14,9 @@ import { ComponentPublicInstance, Ref, cloneVNode, - provide + provide, + VNode, + VNodeNormalizedChildren } from '@vue/runtime-test' import { KeepAliveProps } from '../../src/components/KeepAlive' @@ -567,26 +569,25 @@ describe('KeepAlive', () => { }) }) - it('should do router view', async () => { + it('should not call onVnodeUnmounted', async () => { const Foo = markRaw({ name: 'Foo', - template: ` -
This is Foo
- ` + render() { + return h('Foo') + } }) - const Bar = markRaw({ name: 'Bar', - template: ` -
This is Bar
- ` + render() { + return h('Bar') + } }) const spyMounted = jest.fn() const spyUnmounted = jest.fn() const RouterView = defineComponent({ - setup(props, { slots }) { + setup(_, { slots }) { const Component = inject>('component') const refView = ref() @@ -594,11 +595,9 @@ describe('KeepAlive', () => { ref: refView, onVnodeMounted() { spyMounted() - console.log('mounted ref', refView.value) }, onVnodeUnmounted() { spyUnmounted() - console.log('unmounted ref', refView.value) } } @@ -607,11 +606,8 @@ describe('KeepAlive', () => { Component: Component!.value })[0] - const innerChild = child.children[0] as any - + const innerChild = child.children[0] child.children[0] = cloneVNode(innerChild, componentProps) - console.log('render') - return child } } @@ -620,11 +616,6 @@ describe('KeepAlive', () => { let toggle: () => void = () => {} const App = defineComponent({ - // template: ` - // - // - // - // `, setup() { const component = ref(Foo) @@ -633,22 +624,15 @@ describe('KeepAlive', () => { toggle = () => { component.value = component.value === Foo ? Bar : Foo } - return { component, toggle } - // return () => { - // return h(RouterView, null, h(KeepAlive, null, h(component.value))) - // } }, render() { return h(RouterView, null, { default: ({ Component }: any) => h(KeepAlive, null, [h(Component)]) }) - }, - components: { - RouterView } }) @@ -662,5 +646,13 @@ describe('KeepAlive', () => { expect(spyMounted).toHaveBeenCalledTimes(2) expect(spyUnmounted).toHaveBeenCalledTimes(0) + + toggle() + await nextTick() + render(null, root) + await nextTick() + + expect(spyMounted).toHaveBeenCalledTimes(2) + expect(spyUnmounted).toHaveBeenCalledTimes(2) }) }) From a5b0c0632560db73039e76337ae0185e92135833 Mon Sep 17 00:00:00 2001 From: pikax Date: Wed, 29 Apr 2020 11:40:33 +0100 Subject: [PATCH 3/3] test: remove extra imports --- packages/runtime-core/__tests__/components/KeepAlive.spec.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/runtime-core/__tests__/components/KeepAlive.spec.ts b/packages/runtime-core/__tests__/components/KeepAlive.spec.ts index 891f8d61d44..bf901f6b1e6 100644 --- a/packages/runtime-core/__tests__/components/KeepAlive.spec.ts +++ b/packages/runtime-core/__tests__/components/KeepAlive.spec.ts @@ -14,9 +14,7 @@ import { ComponentPublicInstance, Ref, cloneVNode, - provide, - VNode, - VNodeNormalizedChildren + provide } from '@vue/runtime-test' import { KeepAliveProps } from '../../src/components/KeepAlive'