fix(transition/v-show): ensure transition is in persisted mode when used with v-show

fix #4845
close #4852

packages/compiler-dom/__tests__/transforms/Transition.spec.ts

@@ -0,0 +1,166 @@
+import { compile } from '../../src'
+
+describe('Transition multi children warnings', () => {
+  function checkWarning(
+    template: string,
+    shouldWarn: boolean,
+    message = `<Transition> expects exactly one child element or component.`
+  ) {
+    const spy = jest.fn()
+    compile(template.trim(), {
+      hoistStatic: true,
+      transformHoist: null,
+      onError: err => {
+        spy(err.message)
+      }
+    })
+
+    if (shouldWarn) expect(spy).toHaveBeenCalledWith(message)
+    else expect(spy).not.toHaveBeenCalled()
+  }
+
+  test('warns if multiple children', () => {
+    checkWarning(
+      `
+      <transition>
+        <div>hey</div>
+        <div>hey</div>
+      </transition>
+      `,
+      true
+    )
+  })
+
+  test('warns with v-for', () => {
+    checkWarning(
+      `
+      <transition>
+        <div v-for="i in items">hey</div>
+      </transition>
+      `,
+      true
+    )
+  })
+
+  test('warns with multiple v-if + v-for', () => {
+    checkWarning(
+      `
+      <transition>
+        <div v-if="a" v-for="i in items">hey</div>
+        <div v-else v-for="i in items">hey</div>
+      </transition>
+      `,
+      true
+    )
+  })
+
+  test('warns with template v-if', () => {
+    checkWarning(
+      `
+      <transition>
+        <template v-if="ok"></template>
+      </transition>
+      `,
+      true
+    )
+  })
+
+  test('warns with multiple templates', () => {
+    checkWarning(
+      `
+      <transition>
+        <template v-if="a"></template>
+        <template v-else></template>
+      </transition>
+      `,
+      true
+    )
+  })
+
+  test('warns if multiple children with v-if', () => {
+    checkWarning(
+      `
+      <transition>
+        <div v-if="one">hey</div>
+        <div v-if="other">hey</div>
+      </transition>
+      `,
+      true
+    )
+  })
+
+  test('does not warn with regular element', () => {
+    checkWarning(
+      `
+      <transition>
+        <div>hey</div>
+      </transition>
+      `,
+      false
+    )
+  })
+
+  test('does not warn with one single v-if', () => {
+    checkWarning(
+      `
+      <transition>
+        <div v-if="a">hey</div>
+      </transition>
+      `,
+      false
+    )
+  })
+
+  test('does not warn with v-if v-else-if v-else', () => {
+    checkWarning(
+      `
+      <transition>
+        <div v-if="a">hey</div>
+        <div v-else-if="b">hey</div>
+        <div v-else>hey</div>
+      </transition>
+      `,
+      false
+    )
+  })
+
+  test('does not warn with v-if v-else', () => {
+    checkWarning(
+      `
+      <transition>
+        <div v-if="a">hey</div>
+        <div v-else>hey</div>
+      </transition>
+      `,
+      false
+    )
+  })
+})
+
+test('inject persisted when child has v-show', () => {
+  expect(
+    compile(`
+    <transition>
+      <div v-show="ok" />
+    </transition>
+    `).code
+  ).toMatchSnapshot()
+})
+
+test('the v-if/else-if/else branches in Transition should ignore comments', () => {
+  expect(
+    compile(`
+    <transition>
+      <div v-if="a">hey</div>
+      <!-- this should be ignored -->
+      <div v-else-if="b">hey</div>
+      <!-- this should be ignored -->
+      <div v-else>
+        <p v-if="c"/>
+        <!-- this should not be ignored -->
+        <p v-else/>
+      </div>
+    </transition>
+    `).code
+  ).toMatchSnapshot()
+})

packages/compiler-dom/__tests__/transforms/__snapshots__/warnTransitionChildren.spec.ts.snap → packages/compiler-dom/__tests__/transforms/__snapshots__/Transition.spec.ts.snap

@@ -1,5 +1,24 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`inject persisted when child has v-show 1`] = `
+"const _Vue = Vue
+
+return function render(_ctx, _cache) {
+  with (_ctx) {
+    const { vShow: _vShow, createElementVNode: _createElementVNode, withDirectives: _withDirectives, Transition: _Transition, withCtx: _withCtx, openBlock: _openBlock, createBlock: _createBlock } = _Vue
+
+    return (_openBlock(), _createBlock(_Transition, { persisted: \\"\\" }, {
+      default: _withCtx(() => [
+        _withDirectives(_createElementVNode(\\"div\\", null, null, 512 /* NEED_PATCH */), [
+          [_vShow, ok]
+        ])
+      ]),
+      _: 1 /* STABLE */
+    }))
+  }
+}"
+`;
+
 exports[`the v-if/else-if/else branches in Transition should ignore comments 1`] = `
 "const _Vue = Vue
 

packages/compiler-dom/__tests__/transforms/warnTransitionChildren.spec.ts

@@ -1,158 +0,0 @@
-import { compile } from '../../src'
-
-describe('compiler warnings', () => {
-  describe('Transition', () => {
-    function checkWarning(
-      template: string,
-      shouldWarn: boolean,
-      message = `<Transition> expects exactly one child element or component.`
-    ) {
-      const spy = jest.fn()
-      compile(template.trim(), {
-        hoistStatic: true,
-        transformHoist: null,
-        onError: err => {
-          spy(err.message)
-        }
-      })
-
-      if (shouldWarn) expect(spy).toHaveBeenCalledWith(message)
-      else expect(spy).not.toHaveBeenCalled()
-    }
-
-    test('warns if multiple children', () => {
-      checkWarning(
-        `
-      <transition>
-        <div>hey</div>
-        <div>hey</div>
-      </transition>
-      `,
-        true
-      )
-    })
-
-    test('warns with v-for', () => {
-      checkWarning(
-        `
-      <transition>
-        <div v-for="i in items">hey</div>
-      </transition>
-      `,
-        true
-      )
-    })
-
-    test('warns with multiple v-if + v-for', () => {
-      checkWarning(
-        `
-      <transition>
-        <div v-if="a" v-for="i in items">hey</div>
-        <div v-else v-for="i in items">hey</div>
-      </transition>
-      `,
-        true
-      )
-    })
-
-    test('warns with template v-if', () => {
-      checkWarning(
-        `
-      <transition>
-        <template v-if="ok"></template>
-      </transition>
-      `,
-        true
-      )
-    })
-
-    test('warns with multiple templates', () => {
-      checkWarning(
-        `
-      <transition>
-        <template v-if="a"></template>
-        <template v-else></template>
-      </transition>
-      `,
-        true
-      )
-    })
-
-    test('warns if multiple children with v-if', () => {
-      checkWarning(
-        `
-      <transition>
-        <div v-if="one">hey</div>
-        <div v-if="other">hey</div>
-      </transition>
-      `,
-        true
-      )
-    })
-
-    test('does not warn with regular element', () => {
-      checkWarning(
-        `
-      <transition>
-        <div>hey</div>
-      </transition>
-      `,
-        false
-      )
-    })
-
-    test('does not warn with one single v-if', () => {
-      checkWarning(
-        `
-      <transition>
-        <div v-if="a">hey</div>
-      </transition>
-      `,
-        false
-      )
-    })
-
-    test('does not warn with v-if v-else-if v-else', () => {
-      checkWarning(
-        `
-      <transition>
-        <div v-if="a">hey</div>
-        <div v-else-if="b">hey</div>
-        <div v-else>hey</div>
-      </transition>
-      `,
-        false
-      )
-    })
-
-    test('does not warn with v-if v-else', () => {
-      checkWarning(
-        `
-      <transition>
-        <div v-if="a">hey</div>
-        <div v-else>hey</div>
-      </transition>
-      `,
-        false
-      )
-    })
-  })
-})
-
-test('the v-if/else-if/else branches in Transition should ignore comments', () => {
-  expect(
-    compile(`
-    <transition>
-      <div v-if="a">hey</div>
-      <!-- this should be ignored -->
-      <div v-else-if="b">hey</div>
-      <!-- this should be ignored -->
-      <div v-else>
-        <p v-if="c"/>
-        <!-- this should not be ignored -->
-        <p v-else/>
-      </div>
-    </transition>
-    `).code
-  ).toMatchSnapshot()
-})

packages/compiler-dom/src/index.ts

@@ -16,7 +16,7 @@ import { transformVText } from './transforms/vText'
 import { transformModel } from './transforms/vModel'
 import { transformOn } from './transforms/vOn'
 import { transformShow } from './transforms/vShow'
-import { warnTransitionChildren } from './transforms/warnTransitionChildren'
+import { transformTransition } from './transforms/Transition'
 import { stringifyStatic } from './transforms/stringifyStatic'
 import { ignoreSideEffectTags } from './transforms/ignoreSideEffectTags'
 import { extend } from '@vue/shared'
@@ -25,11 +25,11 @@ export { parserOptions }
 
 export const DOMNodeTransforms: NodeTransform[] = [
   transformStyle,
-  ...(__DEV__ ? [warnTransitionChildren] : [])
+  ...(__DEV__ ? [transformTransition] : [])
 ]
 
 export const DOMDirectiveTransforms: Record<string, DirectiveTransform> = {
-  cloak: noopDirectiveTransform,
+cloak: noopDirectiveTransform,
   html: transformVHtml,
   text: transformVText,
   model: transformModel, // override compiler-core

packages/compiler-dom/src/transforms/warnTransitionChildren.ts → packages/compiler-dom/src/transforms/Transition.ts

@@ -8,7 +8,7 @@ import {
 import { TRANSITION } from '../runtimeHelpers'
 import { createDOMCompilerError, DOMErrorCodes } from '../errors'
 
-export const warnTransitionChildren: NodeTransform = (node, context) => {
+export const transformTransition: NodeTransform = (node, context) => {
   if (
     node.type === NodeTypes.ELEMENT &&
     node.tagType === ElementTypes.COMPONENT
@@ -16,7 +16,12 @@ export const warnTransitionChildren: NodeTransform = (node, context) => {
     const component = context.isBuiltInComponent(node.tag)
     if (component === TRANSITION) {
       return () => {
-        if (node.children.length && hasMultipleChildren(node)) {
+        if (!node.children.length) {
+          return
+        }
+
+        // warn multiple transition children
+        if (hasMultipleChildren(node)) {
           context.onError(
             createDOMCompilerError(
               DOMErrorCodes.X_TRANSITION_INVALID_CHILDREN,
@@ -28,6 +33,22 @@ export const warnTransitionChildren: NodeTransform = (node, context) => {
             )
           )
         }
+
+        // check if it's s single child w/ v-show
+        // if yes, inject "persisted: true" to the transition props
+        const child = node.children[0]
+        if (child.type === NodeTypes.ELEMENT) {
+          for (const p of child.props) {
+            if (p.type === NodeTypes.DIRECTIVE && p.name === 'show') {
+              node.props.push({
+                type: NodeTypes.ATTRIBUTE,
+                name: 'persisted',
+                value: undefined,
+                loc: node.loc
+              })
+            }
+          }
+        }
       }
     }
   }

packages/runtime-core/src/renderer.ts

@@ -713,6 +713,7 @@ function baseCreateRenderer(
       (!parentSuspense || (parentSuspense && !parentSuspense.pendingBranch)) &&
       transition &&
       !transition.persisted
+    if (transition) debugger
     if (needCallTransitionHooks) {
       transition!.beforeEnter(el)
     }

packages/vue/__tests__/Transition.spec.ts

@@ -1614,8 +1614,17 @@ describe('e2e: Transition', () => {
     test(
       'transition on appear with v-show',
       async () => {
+        const beforeEnterSpy = jest.fn()
+        const onEnterSpy = jest.fn()
+        const afterEnterSpy = jest.fn()
+
+        await page().exposeFunction('onEnterSpy', onEnterSpy)
+        await page().exposeFunction('beforeEnterSpy', beforeEnterSpy)
+        await page().exposeFunction('afterEnterSpy', afterEnterSpy)
+
         const appearClass = await page().evaluate(async () => {
           const { createApp, ref } = (window as any).Vue
+          const { beforeEnterSpy, onEnterSpy, afterEnterSpy } = window as any
           createApp({
             template: `
               <div id="container">
@@ -1623,7 +1632,10 @@ describe('e2e: Transition', () => {
                             appear
                             appear-from-class="test-appear-from"
                             appear-to-class="test-appear-to"
-                            appear-active-class="test-appear-active">
+                            appear-active-class="test-appear-active"
+                            @before-enter="beforeEnterSpy"
+                            @enter="onEnterSpy"
+                            @after-enter="afterEnterSpy">
                   <div v-show="toggle" class="test">content</div>
                 </transition>
               </div>
@@ -1632,13 +1644,24 @@ describe('e2e: Transition', () => {
             setup: () => {
               const toggle = ref(true)
               const click = () => (toggle.value = !toggle.value)
-              return { toggle, click }
+              return {
+                toggle,
+                click,
+                beforeEnterSpy,
+                onEnterSpy,
+                afterEnterSpy
+              }
             }
           }).mount('#app')
           return Promise.resolve().then(() => {
             return document.querySelector('.test')!.className.split(/\s+/g)
           })
         })
+
+        expect(beforeEnterSpy).toBeCalledTimes(1)
+        expect(onEnterSpy).toBeCalledTimes(1)
+        expect(afterEnterSpy).toBeCalledTimes(0)
+
         // appear
         expect(appearClass).toStrictEqual([
           'test',
@@ -1654,6 +1677,10 @@ describe('e2e: Transition', () => {
         await transitionFinish()
         expect(await html('#container')).toBe('<div class="test">content</div>')
 
+        expect(beforeEnterSpy).toBeCalledTimes(1)
+        expect(onEnterSpy).toBeCalledTimes(1)
+        expect(afterEnterSpy).toBeCalledTimes(1)
+
         // leave
         expect(await classWhenTransitionStart()).toStrictEqual([
           'test',
@@ -1688,6 +1715,79 @@ describe('e2e: Transition', () => {
       },
       E2E_TIMEOUT
     )
+
+    // #4845
+    test(
+      'transition events should not call onEnter with v-show false',
+      async () => {
+        const beforeEnterSpy = jest.fn()
+        const onEnterSpy = jest.fn()
+        const afterEnterSpy = jest.fn()
+
+        await page().exposeFunction('onEnterSpy', onEnterSpy)
+        await page().exposeFunction('beforeEnterSpy', beforeEnterSpy)
+        await page().exposeFunction('afterEnterSpy', afterEnterSpy)
+
+        await page().evaluate(() => {
+          const { beforeEnterSpy, onEnterSpy, afterEnterSpy } = window as any
+          const { createApp, ref } = (window as any).Vue
+          createApp({
+            template: `
+            <div id="container">
+              <transition
+                name="test"
+                appear
+                @before-enter="beforeEnterSpy"
+                @enter="onEnterSpy"
+                @after-enter="afterEnterSpy">
+                <div v-show="toggle" class="test">content</div>
+              </transition>
+            </div>
+            <button id="toggleBtn" @click="click">button</button>
+          `,
+            setup: () => {
+              const toggle = ref(false)
+              const click = () => (toggle.value = !toggle.value)
+              return {
+                toggle,
+                click,
+                beforeEnterSpy,
+                onEnterSpy,
+                afterEnterSpy
+              }
+            }
+          }).mount('#app')
+        })
+        await nextTick()
+
+        expect(await isVisible('.test')).toBe(false)
+
+        expect(beforeEnterSpy).toBeCalledTimes(0)
+        expect(onEnterSpy).toBeCalledTimes(0)
+        // enter
+        expect(await classWhenTransitionStart()).toStrictEqual([
+          'test',
+          'test-enter-from',
+          'test-enter-active'
+        ])
+        expect(beforeEnterSpy).toBeCalledTimes(1)
+        expect(onEnterSpy).toBeCalledTimes(1)
+        expect(afterEnterSpy).not.toBeCalled()
+        await nextFrame()
+        expect(await classList('.test')).toStrictEqual([
+          'test',
+          'test-enter-active',
+          'test-enter-to'
+        ])
+        expect(afterEnterSpy).not.toBeCalled()
+        await transitionFinish()
+        expect(await html('#container')).toBe(
+          '<div class="test" style="">content</div>'
+        )
+        expect(afterEnterSpy).toBeCalled()
+      },
+      E2E_TIMEOUT
+    )
   })
 
   describe('explicit durations', () => {

scripts/filter-e2e.js

@@ -1,4 +1,8 @@
-const e2eTests = ['/Transition', '/TransitionGroup', '/examples/']
+const e2eTests = [
+  'vue/__tests__/Transition',
+  'vue/__tests__/TransitionGroup',
+  'vue/examples/'
+]
 
 module.exports = list => {
   return {