fix(transition): enter/leave hook timing consistency with v2

close #1145

packages/runtime-dom/src/components/Transition.ts

@@ -3,12 +3,9 @@ import {
   BaseTransitionProps,
   h,
   warn,
-  FunctionalComponent,
-  getCurrentInstance,
-  callWithAsyncErrorHandling
+  FunctionalComponent
 } from '@vue/runtime-core'
 import { isObject, toNumber, extend } from '@vue/shared'
-import { ErrorCodes } from 'packages/runtime-core/src/errorHandling'
 
 const TRANSITION = 'transition'
 const ANIMATION = 'animation'
@@ -94,7 +91,6 @@ export function resolveTransitionProps(
     return baseProps
   }
 
-  const instance = getCurrentInstance()!
   const durations = normalizeDuration(duration)
   const enterDuration = durations && durations[0]
   const leaveDuration = durations && durations[1]
@@ -104,14 +100,11 @@ export function resolveTransitionProps(
     onEnterCancelled,
     onLeave,
     onLeaveCancelled,
-    onBeforeAppear,
-    onAppear,
-    onAppearCancelled
+    onBeforeAppear = onBeforeEnter,
+    onAppear = onEnter,
+    onAppearCancelled = onEnterCancelled
   } = baseProps
 
-  type HookWithDone = (el: Element, done: () => void) => void
-  type Hook = HookWithDone | ((el: Element) => void)
-
   const finishEnter = (el: Element, isAppear: boolean, done?: () => void) => {
     removeTransitionClass(el, isAppear ? appearToClass : enterToClass)
     removeTransitionClass(el, isAppear ? appearActiveClass : enterActiveClass)
@@ -124,24 +117,12 @@ export function resolveTransitionProps(
     done && done()
   }
 
-  // only needed for user hooks called in nextFrame
-  // sync errors are already handled by BaseTransition
-  const callHook = (hook: Hook | undefined, args: any[]) => {
-    hook &&
-      callWithAsyncErrorHandling(
-        hook,
-        instance,
-        ErrorCodes.TRANSITION_HOOK,
-        args
-      )
-  }
-
-  const makeEnterHook = (isAppear: boolean): HookWithDone => {
-    const hook = isAppear ? onAppear : onEnter
-    return (el, done) => {
+  const makeEnterHook = (isAppear: boolean) => {
+    return (el: Element, done: () => void) => {
+      const hook = isAppear ? onAppear : onEnter
+      const resolve = () => finishEnter(el, isAppear, done)
+      hook && hook(el, resolve)
       nextFrame(() => {
-        const resolve = () => finishEnter(el, isAppear, done)
-        callHook(hook, [el, resolve])
         removeTransitionClass(el, isAppear ? appearFromClass : enterFromClass)
         addTransitionClass(el, isAppear ? appearToClass : enterToClass)
         if (!(hook && hook.length > 1)) {
@@ -169,11 +150,10 @@ export function resolveTransitionProps(
     onEnter: makeEnterHook(false),
     onAppear: makeEnterHook(true),
     onLeave(el, done) {
+      const resolve = () => finishLeave(el, done)
       addTransitionClass(el, leaveActiveClass)
       addTransitionClass(el, leaveFromClass)
       nextFrame(() => {
-        const resolve = () => finishLeave(el, done)
-        callHook(onLeave, [el, resolve])
         removeTransitionClass(el, leaveFromClass)
         addTransitionClass(el, leaveToClass)
         if (!(onLeave && onLeave.length > 1)) {
@@ -184,6 +164,7 @@ export function resolveTransitionProps(
           }
         }
       })
+      onLeave && onLeave(el, resolve)
     },
     onEnterCancelled(el) {
       finishEnter(el, false)

packages/vue/__tests__/Transition.spec.ts

@@ -339,7 +339,7 @@ describe('e2e: Transition', () => {
         ])
         // todo test event with arguments. Note: not get dom, get object. '{}'
         expect(beforeLeaveSpy).toBeCalled()
-        expect(onLeaveSpy).not.toBeCalled()
+        expect(onLeaveSpy).toBeCalled()
         expect(afterLeaveSpy).not.toBeCalled()
         await nextFrame()
         expect(await classList('.test')).toStrictEqual([
@@ -347,7 +347,6 @@ describe('e2e: Transition', () => {
           'test-leave-active',
           'test-leave-to'
         ])
-        expect(beforeLeaveSpy).toBeCalled()
         expect(afterLeaveSpy).not.toBeCalled()
         await transitionFinish()
         expect(await html('#container')).toBe('<!--v-if-->')
@@ -360,7 +359,7 @@ describe('e2e: Transition', () => {
           'test-enter-from'
         ])
         expect(beforeEnterSpy).toBeCalled()
-        expect(onEnterSpy).not.toBeCalled()
+        expect(onEnterSpy).toBeCalled()
         expect(afterEnterSpy).not.toBeCalled()
         await nextFrame()
         expect(await classList('.test')).toStrictEqual([
@@ -368,7 +367,6 @@ describe('e2e: Transition', () => {
           'test-enter-active',
           'test-enter-to'
         ])
-        expect(onEnterSpy).toBeCalled()
         expect(afterEnterSpy).not.toBeCalled()
         await transitionFinish()
         expect(await html('#container')).toBe('<div class="test">content</div>')
@@ -603,7 +601,7 @@ describe('e2e: Transition', () => {
           'test-appear-from'
         ])
         expect(beforeAppearSpy).toBeCalled()
-        expect(onAppearSpy).not.toBeCalled()
+        expect(onAppearSpy).toBeCalled()
         expect(afterAppearSpy).not.toBeCalled()
         await nextFrame()
         expect(await classList('.test')).toStrictEqual([
@@ -611,7 +609,6 @@ describe('e2e: Transition', () => {
           'test-appear-active',
           'test-appear-to'
         ])
-        expect(onAppearSpy).toBeCalled()
         expect(afterAppearSpy).not.toBeCalled()
         await transitionFinish()
         expect(await html('#container')).toBe('<div class="test">content</div>')
@@ -628,7 +625,7 @@ describe('e2e: Transition', () => {
           'test-leave-from'
         ])
         expect(beforeLeaveSpy).toBeCalled()
-        expect(onLeaveSpy).not.toBeCalled()
+        expect(onLeaveSpy).toBeCalled()
         expect(afterLeaveSpy).not.toBeCalled()
         await nextFrame()
         expect(await classList('.test')).toStrictEqual([
@@ -636,7 +633,6 @@ describe('e2e: Transition', () => {
           'test-leave-active',
           'test-leave-to'
         ])
-        expect(onLeaveSpy).toBeCalled()
         expect(afterLeaveSpy).not.toBeCalled()
         await transitionFinish()
         expect(await html('#container')).toBe('<!--v-if-->')
@@ -649,7 +645,7 @@ describe('e2e: Transition', () => {
           'test-enter-from'
         ])
         expect(beforeEnterSpy).toBeCalled()
-        expect(onEnterSpy).not.toBeCalled()
+        expect(onEnterSpy).toBeCalled()
         expect(afterEnterSpy).not.toBeCalled()
         await nextFrame()
         expect(await classList('.test')).toStrictEqual([
@@ -657,7 +653,6 @@ describe('e2e: Transition', () => {
           'test-enter-active',
           'test-enter-to'
         ])
-        expect(onEnterSpy).toBeCalled()
         expect(afterEnterSpy).not.toBeCalled()
         await transitionFinish()
         expect(await html('#container')).toBe('<div class="test">content</div>')
@@ -1233,7 +1228,7 @@ describe('e2e: Transition', () => {
           'test-leave-from'
         ])
         expect(beforeLeaveSpy).toBeCalled()
-        expect(onLeaveSpy).not.toBeCalled()
+        expect(onLeaveSpy).toBeCalled()
         expect(afterLeaveSpy).not.toBeCalled()
         await nextFrame()
         expect(await classList('.test')).toStrictEqual([
@@ -1241,7 +1236,6 @@ describe('e2e: Transition', () => {
           'test-leave-active',
           'test-leave-to'
         ])
-        expect(beforeLeaveSpy).toBeCalled()
         expect(afterLeaveSpy).not.toBeCalled()
         await transitionFinish()
         expect(await isVisible('.test')).toBe(false)
@@ -1254,7 +1248,7 @@ describe('e2e: Transition', () => {
           'test-enter-from'
         ])
         expect(beforeEnterSpy).toBeCalled()
-        expect(onEnterSpy).not.toBeCalled()
+        expect(onEnterSpy).toBeCalled()
         expect(afterEnterSpy).not.toBeCalled()
         await nextFrame()
         expect(await classList('.test')).toStrictEqual([
@@ -1262,7 +1256,6 @@ describe('e2e: Transition', () => {
           'test-enter-active',
           'test-enter-to'
         ])
-        expect(onEnterSpy).toBeCalled()
         expect(afterEnterSpy).not.toBeCalled()
         await transitionFinish()
         expect(await html('#container')).toBe(

packages/vue/__tests__/TransitionGroup.spec.ts

@@ -439,7 +439,7 @@ describe('e2e: TransitionGroup', () => {
         })
       })
       expect(beforeAppearSpy).toBeCalled()
-      expect(onAppearSpy).not.toBeCalled()
+      expect(onAppearSpy).toBeCalled()
       expect(afterAppearSpy).not.toBeCalled()
       expect(appearHtml).toBe(
         `<div class="test test-appear-active test-appear-from">a</div>` +
@@ -447,7 +447,6 @@ describe('e2e: TransitionGroup', () => {
           `<div class="test test-appear-active test-appear-from">c</div>`
       )
       await nextFrame()
-      expect(onAppearSpy).toBeCalled()
       expect(afterAppearSpy).not.toBeCalled()
       expect(await html('#container')).toBe(
         `<div class="test test-appear-active test-appear-to">a</div>` +
@@ -470,10 +469,10 @@ describe('e2e: TransitionGroup', () => {
           `<div class="test test-enter-active test-enter-from">d</div>`
       )
       expect(beforeLeaveSpy).toBeCalled()
-      expect(onLeaveSpy).not.toBeCalled()
+      expect(onLeaveSpy).toBeCalled()
       expect(afterLeaveSpy).not.toBeCalled()
       expect(beforeEnterSpy).toBeCalled()
-      expect(onEnterSpy).not.toBeCalled()
+      expect(onEnterSpy).toBeCalled()
       expect(afterEnterSpy).not.toBeCalled()
       await nextFrame()
       expect(await html('#container')).toBe(
@@ -482,9 +481,7 @@ describe('e2e: TransitionGroup', () => {
           `<div class="test">c</div>` +
           `<div class="test test-enter-active test-enter-to">d</div>`
       )
-      expect(onLeaveSpy).toBeCalled()
       expect(afterLeaveSpy).not.toBeCalled()
-      expect(onEnterSpy).toBeCalled()
       expect(afterEnterSpy).not.toBeCalled()
       await transitionFinish()
       expect(await html('#container')).toBe(