From 1c4f488f03dfe8162d601c3c3541002d58d10cb9 Mon Sep 17 00:00:00 2001 From: daiwei Date: Fri, 26 Apr 2024 16:53:40 +0800 Subject: [PATCH 01/10] fix(runtime-core): properly handle inherit transition during fallthrough attrs --- packages/runtime-core/src/componentRenderUtils.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/runtime-core/src/componentRenderUtils.ts b/packages/runtime-core/src/componentRenderUtils.ts index 31a1a5fa122..ea728d5d3f1 100644 --- a/packages/runtime-core/src/componentRenderUtils.ts +++ b/packages/runtime-core/src/componentRenderUtils.ts @@ -167,6 +167,7 @@ export function renderComponentRoot( ) } root = cloneVNode(root, fallthroughAttrs) + if (root.transition) root.transition = root.transition.clone(root) } else if (__DEV__ && !accessedAttrs && root.type !== Comment) { const allAttrs = Object.keys(attrs) const eventAttrs: string[] = [] From 1eb6f65811cbb4b6c7bc0812549e931577440f19 Mon Sep 17 00:00:00 2001 From: daiwei Date: Fri, 26 Apr 2024 17:54:55 +0800 Subject: [PATCH 02/10] test: add test --- .../runtime-core/src/componentRenderUtils.ts | 1 + packages/vue/__tests__/e2e/Transition.spec.ts | 47 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/packages/runtime-core/src/componentRenderUtils.ts b/packages/runtime-core/src/componentRenderUtils.ts index ea728d5d3f1..dd675f0910d 100644 --- a/packages/runtime-core/src/componentRenderUtils.ts +++ b/packages/runtime-core/src/componentRenderUtils.ts @@ -226,6 +226,7 @@ export function renderComponentRoot( class: cls, style: style, }) + if (root.transition) root.transition = root.transition.clone(root) } } diff --git a/packages/vue/__tests__/e2e/Transition.spec.ts b/packages/vue/__tests__/e2e/Transition.spec.ts index e8d6d1e049e..c5ce7668a8b 100644 --- a/packages/vue/__tests__/e2e/Transition.spec.ts +++ b/packages/vue/__tests__/e2e/Transition.spec.ts @@ -1214,6 +1214,53 @@ describe('e2e: Transition', () => { }, E2E_TIMEOUT, ) + + // #3716 + test( + 'wrapping transition + fallthrough attrs', + async () => { + await page().goto(baseUrl) + await page().waitForSelector('#app') + await page().evaluate(() => { + const { createApp, ref } = (window as any).Vue + createApp({ + components: { + 'my-transition': { + template: ` + + + + `, + }, + }, + template: ` +
+ +
content
+
+
+ + `, + setup: () => { + const toggle = ref(true) + const click = () => (toggle.value = !toggle.value) + return { toggle, click } + }, + }).mount('#app') + }) + expect(await html('#container')).toBe('
content
') + + await click('#toggleBtn') + await nextTick() + await click('#toggleBtn') + + await transitionFinish() + expect(await html('#container')).toBe( + '
content
', + ) + }, + E2E_TIMEOUT, + ) }) describe('transition with Suspense', () => { From 3a5a245184834d68d02ced4077057d5c840455ba Mon Sep 17 00:00:00 2001 From: daiwei Date: Fri, 26 Apr 2024 18:04:26 +0800 Subject: [PATCH 03/10] test: add test --- packages/vue/__tests__/e2e/Transition.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/vue/__tests__/e2e/Transition.spec.ts b/packages/vue/__tests__/e2e/Transition.spec.ts index c5ce7668a8b..bd0960605fe 100644 --- a/packages/vue/__tests__/e2e/Transition.spec.ts +++ b/packages/vue/__tests__/e2e/Transition.spec.ts @@ -1251,6 +1251,7 @@ describe('e2e: Transition', () => { expect(await html('#container')).toBe('
content
') await click('#toggleBtn') + // toggle before leave transition complete await nextTick() await click('#toggleBtn') From 5cecfa76dd9db890158aab29c189871128ffc0be Mon Sep 17 00:00:00 2001 From: edison1105 Date: Sat, 27 Apr 2024 09:40:37 +0800 Subject: [PATCH 04/10] chore: tweaks --- .../runtime-core/src/componentRenderUtils.ts | 19 +++++++++++-------- packages/runtime-core/src/vnode.ts | 5 +++++ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/packages/runtime-core/src/componentRenderUtils.ts b/packages/runtime-core/src/componentRenderUtils.ts index dd675f0910d..7ed39bbe02a 100644 --- a/packages/runtime-core/src/componentRenderUtils.ts +++ b/packages/runtime-core/src/componentRenderUtils.ts @@ -166,8 +166,7 @@ export function renderComponentRoot( propsOptions, ) } - root = cloneVNode(root, fallthroughAttrs) - if (root.transition) root.transition = root.transition.clone(root) + root = cloneVNode(root, fallthroughAttrs, false, true) } else if (__DEV__ && !accessedAttrs && root.type !== Comment) { const allAttrs = Object.keys(attrs) const eventAttrs: string[] = [] @@ -222,11 +221,15 @@ export function renderComponentRoot( getComponentName(instance.type), ) } - root = cloneVNode(root, { - class: cls, - style: style, - }) - if (root.transition) root.transition = root.transition.clone(root) + root = cloneVNode( + root, + { + class: cls, + style: style, + }, + false, + true, + ) } } @@ -239,7 +242,7 @@ export function renderComponentRoot( ) } // clone before mutating since the root may be a hoisted vnode - root = cloneVNode(root) + root = cloneVNode(root, undefined, false, true) root.dirs = root.dirs ? root.dirs.concat(vnode.dirs) : vnode.dirs } // inherit transition data diff --git a/packages/runtime-core/src/vnode.ts b/packages/runtime-core/src/vnode.ts index a1a6a908d2a..9fa2e592158 100644 --- a/packages/runtime-core/src/vnode.ts +++ b/packages/runtime-core/src/vnode.ts @@ -624,6 +624,7 @@ export function cloneVNode( vnode: VNode, extraProps?: (Data & VNodeProps) | null, mergeRef = false, + cloneTransition = false, ): VNode { // This is intentionally NOT using spread or extend to avoid the runtime // key enumeration cost. @@ -688,6 +689,10 @@ export function cloneVNode( if (__COMPAT__) { defineLegacyVNodeProperties(cloned as VNode) } + + if (cloneTransition && vnode.transition) { + cloned.transition = vnode.transition.clone(cloned as VNode) + } return cloned } From e78b958f40dc4d782fd016326cf0f72eea70b88c Mon Sep 17 00:00:00 2001 From: daiwei Date: Sun, 28 Apr 2024 09:08:33 +0800 Subject: [PATCH 05/10] chore: tweaks --- packages/runtime-core/src/vnode.ts | 8 +++++--- packages/vue/__tests__/e2e/Transition.spec.ts | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/runtime-core/src/vnode.ts b/packages/runtime-core/src/vnode.ts index 9fa2e592158..12f70822276 100644 --- a/packages/runtime-core/src/vnode.ts +++ b/packages/runtime-core/src/vnode.ts @@ -671,7 +671,7 @@ export function cloneVNode( dynamicChildren: vnode.dynamicChildren, appContext: vnode.appContext, dirs: vnode.dirs, - transition: vnode.transition, + transition: null, // These should technically only be non-null on mounted VNodes. However, // they *should* be copied for kept-alive vnodes. So we just always copy @@ -690,8 +690,10 @@ export function cloneVNode( defineLegacyVNodeProperties(cloned as VNode) } - if (cloneTransition && vnode.transition) { - cloned.transition = vnode.transition.clone(cloned as VNode) + if (vnode.transition) { + cloned.transition = cloneTransition + ? vnode.transition.clone(cloned as VNode) + : vnode.transition } return cloned } diff --git a/packages/vue/__tests__/e2e/Transition.spec.ts b/packages/vue/__tests__/e2e/Transition.spec.ts index bd0960605fe..1df325955c6 100644 --- a/packages/vue/__tests__/e2e/Transition.spec.ts +++ b/packages/vue/__tests__/e2e/Transition.spec.ts @@ -1251,7 +1251,7 @@ describe('e2e: Transition', () => { expect(await html('#container')).toBe('
content
') await click('#toggleBtn') - // toggle before leave transition complete + // toggle again before leave finishes await nextTick() await click('#toggleBtn') From aef614707cd585da08d4b39b45bad35633e4ed13 Mon Sep 17 00:00:00 2001 From: daiwei Date: Sun, 28 Apr 2024 09:18:25 +0800 Subject: [PATCH 06/10] chore: add comments --- packages/runtime-core/src/vnode.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/runtime-core/src/vnode.ts b/packages/runtime-core/src/vnode.ts index 12f70822276..be193c194a9 100644 --- a/packages/runtime-core/src/vnode.ts +++ b/packages/runtime-core/src/vnode.ts @@ -686,15 +686,20 @@ export function cloneVNode( ctx: vnode.ctx, ce: vnode.ce, } - if (__COMPAT__) { - defineLegacyVNodeProperties(cloned as VNode) - } + // if the vnode will be replaced by the cloned one, it is necessary + // to clone the transition to ensure that the vnode referenced within the transition hooks + // is refresh. if (vnode.transition) { cloned.transition = cloneTransition ? vnode.transition.clone(cloned as VNode) : vnode.transition } + + if (__COMPAT__) { + defineLegacyVNodeProperties(cloned as VNode) + } + return cloned } From 86bc1feaf8a3c654bd93fc52874b68ed14dc8879 Mon Sep 17 00:00:00 2001 From: daiwei Date: Sun, 28 Apr 2024 09:19:16 +0800 Subject: [PATCH 07/10] chore: add comments --- packages/runtime-core/src/vnode.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/runtime-core/src/vnode.ts b/packages/runtime-core/src/vnode.ts index be193c194a9..cfe421332ae 100644 --- a/packages/runtime-core/src/vnode.ts +++ b/packages/runtime-core/src/vnode.ts @@ -688,8 +688,8 @@ export function cloneVNode( } // if the vnode will be replaced by the cloned one, it is necessary - // to clone the transition to ensure that the vnode referenced within the transition hooks - // is refresh. + // to clone the transition to ensure that the vnode referenced within + // the transition hooks is refresh. if (vnode.transition) { cloned.transition = cloneTransition ? vnode.transition.clone(cloned as VNode) From 0d04b9dc2359b286ef1808fcd5ba9f381e5905f1 Mon Sep 17 00:00:00 2001 From: daiwei Date: Sun, 28 Apr 2024 09:20:31 +0800 Subject: [PATCH 08/10] chore: add comments --- packages/runtime-core/src/vnode.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/runtime-core/src/vnode.ts b/packages/runtime-core/src/vnode.ts index cfe421332ae..e2ba10d244f 100644 --- a/packages/runtime-core/src/vnode.ts +++ b/packages/runtime-core/src/vnode.ts @@ -689,7 +689,7 @@ export function cloneVNode( // if the vnode will be replaced by the cloned one, it is necessary // to clone the transition to ensure that the vnode referenced within - // the transition hooks is refresh. + // the transition hooks is fresh. if (vnode.transition) { cloned.transition = cloneTransition ? vnode.transition.clone(cloned as VNode) From 426b1f752da8bd0db51764d85e9dcff069ea0b4b Mon Sep 17 00:00:00 2001 From: Evan You Date: Mon, 29 Apr 2024 14:31:47 +0800 Subject: [PATCH 09/10] refactor: small tweaks --- packages/runtime-core/src/componentRenderUtils.ts | 2 +- packages/runtime-core/src/vnode.ts | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/runtime-core/src/componentRenderUtils.ts b/packages/runtime-core/src/componentRenderUtils.ts index 7ed39bbe02a..6a6641e09d0 100644 --- a/packages/runtime-core/src/componentRenderUtils.ts +++ b/packages/runtime-core/src/componentRenderUtils.ts @@ -242,7 +242,7 @@ export function renderComponentRoot( ) } // clone before mutating since the root may be a hoisted vnode - root = cloneVNode(root, undefined, false, true) + root = cloneVNode(root, null, false, true) root.dirs = root.dirs ? root.dirs.concat(vnode.dirs) : vnode.dirs } // inherit transition data diff --git a/packages/runtime-core/src/vnode.ts b/packages/runtime-core/src/vnode.ts index e2ba10d244f..0e2a4bafcc5 100644 --- a/packages/runtime-core/src/vnode.ts +++ b/packages/runtime-core/src/vnode.ts @@ -628,7 +628,7 @@ export function cloneVNode( ): VNode { // This is intentionally NOT using spread or extend to avoid the runtime // key enumeration cost. - const { props, ref, patchFlag, children } = vnode + const { props, ref, patchFlag, children, transition } = vnode const mergedProps = extraProps ? mergeProps(props || {}, extraProps) : props const cloned: VNode = { __v_isVNode: true, @@ -671,7 +671,7 @@ export function cloneVNode( dynamicChildren: vnode.dynamicChildren, appContext: vnode.appContext, dirs: vnode.dirs, - transition: null, + transition, // These should technically only be non-null on mounted VNodes. However, // they *should* be copied for kept-alive vnodes. So we just always copy @@ -690,10 +690,8 @@ export function cloneVNode( // if the vnode will be replaced by the cloned one, it is necessary // to clone the transition to ensure that the vnode referenced within // the transition hooks is fresh. - if (vnode.transition) { - cloned.transition = cloneTransition - ? vnode.transition.clone(cloned as VNode) - : vnode.transition + if (transition && cloneTransition) { + cloned.transition = transition.clone(cloned as VNode) } if (__COMPAT__) { From 3974b5de0dd6f168b226c069cf25dc8739089dd5 Mon Sep 17 00:00:00 2001 From: Evan You Date: Mon, 29 Apr 2024 14:35:36 +0800 Subject: [PATCH 10/10] chore: resolve conflict --- packages/vue/__tests__/e2e/Transition.spec.ts | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/packages/vue/__tests__/e2e/Transition.spec.ts b/packages/vue/__tests__/e2e/Transition.spec.ts index 1df325955c6..4fe78ae8ab0 100644 --- a/packages/vue/__tests__/e2e/Transition.spec.ts +++ b/packages/vue/__tests__/e2e/Transition.spec.ts @@ -1262,6 +1262,63 @@ describe('e2e: Transition', () => { }, E2E_TIMEOUT, ) + + test( + 'w/ KeepAlive + unmount innerChild', + async () => { + const unmountSpy = vi.fn() + await page().exposeFunction('unmountSpy', unmountSpy) + await page().evaluate(() => { + const { unmountSpy } = window as any + const { createApp, ref, h, onUnmounted } = (window as any).Vue + createApp({ + template: ` +
+ + + + + +
+ + `, + components: { + TrueBranch: { + name: 'TrueBranch', + setup() { + onUnmounted(unmountSpy) + const count = ref(0) + return () => h('div', count.value) + }, + }, + }, + setup: () => { + const includeRef = ref(['TrueBranch']) + const toggle = ref(true) + const click = () => { + toggle.value = !toggle.value + if (toggle.value) { + includeRef.value = ['TrueBranch'] + } else { + includeRef.value = [] + } + } + return { toggle, click, unmountSpy, includeRef } + }, + }).mount('#app') + }) + + await transitionFinish() + expect(await html('#container')).toBe('
0
') + + await click('#toggleBtn') + + await transitionFinish() + expect(await html('#container')).toBe('') + expect(unmountSpy).toBeCalledTimes(1) + }, + E2E_TIMEOUT, + ) }) describe('transition with Suspense', () => {