From 071986a2c6459fd99b91a48793a9ab6d6618b52d Mon Sep 17 00:00:00 2001 From: Evan You Date: Fri, 28 May 2021 15:42:08 -0400 Subject: [PATCH] fix(transition): fix higher order transition components with merged listeners fix #3227 --- .../src/components/BaseTransition.ts | 4 +- .../runtime-dom/src/components/Transition.ts | 49 +++++++++--- packages/vue/__tests__/Transition.spec.ts | 74 ++++++++++++++----- 3 files changed, 97 insertions(+), 30 deletions(-) diff --git a/packages/runtime-core/src/components/BaseTransition.ts b/packages/runtime-core/src/components/BaseTransition.ts index 1cb47c8195e..0a6e302a1f5 100644 --- a/packages/runtime-core/src/components/BaseTransition.ts +++ b/packages/runtime-core/src/components/BaseTransition.ts @@ -69,8 +69,8 @@ export interface TransitionHooks< delayedLeave?(): void } -type TransitionHookCaller = ( - hook: ((el: any) => void) | undefined, +export type TransitionHookCaller = ( + hook: ((el: any) => void) | Array<(el: any) => void> | undefined, args?: any[] ) => void diff --git a/packages/runtime-dom/src/components/Transition.ts b/packages/runtime-dom/src/components/Transition.ts index a253f51b435..e369922fcfa 100644 --- a/packages/runtime-dom/src/components/Transition.ts +++ b/packages/runtime-dom/src/components/Transition.ts @@ -7,7 +7,7 @@ import { compatUtils, DeprecationTypes } from '@vue/runtime-core' -import { isObject, toNumber, extend } from '@vue/shared' +import { isObject, toNumber, extend, isArray } from '@vue/shared' const TRANSITION = 'transition' const ANIMATION = 'animation' @@ -75,6 +75,35 @@ export const TransitionPropsValidators = (Transition.props = /*#__PURE__*/ exten DOMTransitionPropsValidators )) +/** + * #3227 Incoming hooks may be merged into arrays when wrapping Transition + * with custom HOCs. + */ +const callHook = ( + hook: Function | Function[] | undefined, + args: any[] = [] +) => { + if (isArray(hook)) { + hook.forEach(h => h(...args)) + } else if (hook) { + hook(...args) + } +} + +/** + * Check if a hook expects a callback (2nd arg), which means the user + * intends to explicitly control the end of the transition. + */ +const hasExplicitCallback = ( + hook: Function | Function[] | undefined +): boolean => { + return hook + ? isArray(hook) + ? hook.some(h => h.length > 1) + : hook.length > 1 + : false +} + export function resolveTransitionProps( rawProps: TransitionProps ): BaseTransitionProps { @@ -154,7 +183,7 @@ export function resolveTransitionProps( return (el: Element, done: () => void) => { const hook = isAppear ? onAppear : onEnter const resolve = () => finishEnter(el, isAppear, done) - hook && hook(el, resolve) + callHook(hook, [el, resolve]) nextFrame(() => { removeTransitionClass(el, isAppear ? appearFromClass : enterFromClass) if (__COMPAT__ && legacyClassEnabled) { @@ -164,7 +193,7 @@ export function resolveTransitionProps( ) } addTransitionClass(el, isAppear ? appearToClass : enterToClass) - if (!(hook && hook.length > 1)) { + if (!hasExplicitCallback(hook)) { whenTransitionEnds(el, type, enterDuration, resolve) } }) @@ -173,7 +202,7 @@ export function resolveTransitionProps( return extend(baseProps, { onBeforeEnter(el) { - onBeforeEnter && onBeforeEnter(el) + callHook(onBeforeEnter, [el]) addTransitionClass(el, enterFromClass) if (__COMPAT__ && legacyClassEnabled) { addTransitionClass(el, legacyEnterFromClass) @@ -181,7 +210,7 @@ export function resolveTransitionProps( addTransitionClass(el, enterActiveClass) }, onBeforeAppear(el) { - onBeforeAppear && onBeforeAppear(el) + callHook(onBeforeAppear, [el]) addTransitionClass(el, appearFromClass) if (__COMPAT__ && legacyClassEnabled) { addTransitionClass(el, legacyAppearFromClass) @@ -205,23 +234,23 @@ export function resolveTransitionProps( removeTransitionClass(el, legacyLeaveFromClass) } addTransitionClass(el, leaveToClass) - if (!(onLeave && onLeave.length > 1)) { + if (!hasExplicitCallback(onLeave)) { whenTransitionEnds(el, type, leaveDuration, resolve) } }) - onLeave && onLeave(el, resolve) + callHook(onLeave, [el, resolve]) }, onEnterCancelled(el) { finishEnter(el, false) - onEnterCancelled && onEnterCancelled(el) + callHook(onEnterCancelled, [el]) }, onAppearCancelled(el) { finishEnter(el, true) - onAppearCancelled && onAppearCancelled(el) + callHook(onAppearCancelled, [el]) }, onLeaveCancelled(el) { finishLeave(el) - onLeaveCancelled && onLeaveCancelled(el) + callHook(onLeaveCancelled, [el]) } } as BaseTransitionProps) } diff --git a/packages/vue/__tests__/Transition.spec.ts b/packages/vue/__tests__/Transition.spec.ts index 5f9a30fda52..eb56c4fd39b 100644 --- a/packages/vue/__tests__/Transition.spec.ts +++ b/packages/vue/__tests__/Transition.spec.ts @@ -1,6 +1,6 @@ import { E2E_TIMEOUT, setupPuppeteer } from './e2eUtils' import path from 'path' -import { h, createApp, Transition } from 'vue' +import { h, createApp, Transition, ref, nextTick } from 'vue' describe('e2e: Transition', () => { const { @@ -1634,23 +1634,6 @@ describe('e2e: Transition', () => { ) }) - test( - 'warn when used on multiple elements', - async () => { - createApp({ - render() { - return h(Transition, null, { - default: () => [h('div'), h('div')] - }) - } - }).mount(document.createElement('div')) - expect( - ' can only be used on a single element or component' - ).toHaveBeenWarned() - }, - E2E_TIMEOUT - ) - describe('explicit durations', () => { test( 'single value', @@ -1916,4 +1899,59 @@ describe('e2e: Transition', () => { E2E_TIMEOUT ) }) + + test('warn when used on multiple elements', async () => { + createApp({ + render() { + return h(Transition, null, { + default: () => [h('div'), h('div')] + }) + } + }).mount(document.createElement('div')) + expect( + ' can only be used on a single element or component' + ).toHaveBeenWarned() + }) + + // #3227 + test(`HOC w/ merged hooks`, async () => { + const innerSpy = jest.fn() + const outerSpy = jest.fn() + + const MyTransition = { + render(this: any) { + return h( + Transition, + { + onLeave(el, end) { + innerSpy() + end() + } + }, + this.$slots.default + ) + } + } + + const toggle = ref(true) + + const root = document.createElement('div') + createApp({ + render() { + return h( + MyTransition, + { onLeave: () => outerSpy() }, + () => (toggle.value ? h('div') : null) + ) + } + }).mount(root) + + expect(root.innerHTML).toBe(`
`) + + toggle.value = false + await nextTick() + expect(innerSpy).toHaveBeenCalledTimes(1) + expect(outerSpy).toHaveBeenCalledTimes(1) + expect(root.innerHTML).toBe(``) + }) })