From 425310e8b614ad91660dd93d4e7905fbe572ef31 Mon Sep 17 00:00:00 2001 From: Evan You Date: Wed, 11 May 2022 19:22:55 +0800 Subject: [PATCH] fix(transition/v-show): ensure transition is in persisted mode when used with v-show fix #4845 close #4852 --- .../__tests__/transforms/Transition.spec.ts | 166 ++++++++++++++++++ ...n.spec.ts.snap => Transition.spec.ts.snap} | 19 ++ .../transforms/warnTransitionChildren.spec.ts | 158 ----------------- packages/compiler-dom/src/index.ts | 6 +- ...arnTransitionChildren.ts => Transition.ts} | 25 ++- packages/runtime-core/src/renderer.ts | 1 + packages/vue/__tests__/Transition.spec.ts | 104 ++++++++++- scripts/filter-e2e.js | 6 +- 8 files changed, 319 insertions(+), 166 deletions(-) create mode 100644 packages/compiler-dom/__tests__/transforms/Transition.spec.ts rename packages/compiler-dom/__tests__/transforms/__snapshots__/{warnTransitionChildren.spec.ts.snap => Transition.spec.ts.snap} (67%) delete mode 100644 packages/compiler-dom/__tests__/transforms/warnTransitionChildren.spec.ts rename packages/compiler-dom/src/transforms/{warnTransitionChildren.ts => Transition.ts} (62%) diff --git a/packages/compiler-dom/__tests__/transforms/Transition.spec.ts b/packages/compiler-dom/__tests__/transforms/Transition.spec.ts new file mode 100644 index 00000000..1ae71360 --- /dev/null +++ b/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 = ` 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( + ` + +
hey
+
hey
+ + `, + true + ) + }) + + test('warns with v-for', () => { + checkWarning( + ` + +
hey
+ + `, + true + ) + }) + + test('warns with multiple v-if + v-for', () => { + checkWarning( + ` + +
hey
+
hey
+ + `, + true + ) + }) + + test('warns with template v-if', () => { + checkWarning( + ` + + + + `, + true + ) + }) + + test('warns with multiple templates', () => { + checkWarning( + ` + + + + + `, + true + ) + }) + + test('warns if multiple children with v-if', () => { + checkWarning( + ` + +
hey
+
hey
+ + `, + true + ) + }) + + test('does not warn with regular element', () => { + checkWarning( + ` + +
hey
+ + `, + false + ) + }) + + test('does not warn with one single v-if', () => { + checkWarning( + ` + +
hey
+ + `, + false + ) + }) + + test('does not warn with v-if v-else-if v-else', () => { + checkWarning( + ` + +
hey
+
hey
+
hey
+ + `, + false + ) + }) + + test('does not warn with v-if v-else', () => { + checkWarning( + ` + +
hey
+
hey
+ + `, + false + ) + }) +}) + +test('inject persisted when child has v-show', () => { + expect( + compile(` + +
+ + `).code + ).toMatchSnapshot() +}) + +test('the v-if/else-if/else branches in Transition should ignore comments', () => { + expect( + compile(` + +
hey
+ +
hey
+ +
+

+ +

+

+ + `).code + ).toMatchSnapshot() +}) diff --git a/packages/compiler-dom/__tests__/transforms/__snapshots__/warnTransitionChildren.spec.ts.snap b/packages/compiler-dom/__tests__/transforms/__snapshots__/Transition.spec.ts.snap similarity index 67% rename from packages/compiler-dom/__tests__/transforms/__snapshots__/warnTransitionChildren.spec.ts.snap rename to packages/compiler-dom/__tests__/transforms/__snapshots__/Transition.spec.ts.snap index 2b39b25f..026fb8aa 100644 --- a/packages/compiler-dom/__tests__/transforms/__snapshots__/warnTransitionChildren.spec.ts.snap +++ b/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 diff --git a/packages/compiler-dom/__tests__/transforms/warnTransitionChildren.spec.ts b/packages/compiler-dom/__tests__/transforms/warnTransitionChildren.spec.ts deleted file mode 100644 index b037af8e..00000000 --- a/packages/compiler-dom/__tests__/transforms/warnTransitionChildren.spec.ts +++ /dev/null @@ -1,158 +0,0 @@ -import { compile } from '../../src' - -describe('compiler warnings', () => { - describe('Transition', () => { - function checkWarning( - template: string, - shouldWarn: boolean, - message = ` 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( - ` - -
hey
-
hey
- - `, - true - ) - }) - - test('warns with v-for', () => { - checkWarning( - ` - -
hey
- - `, - true - ) - }) - - test('warns with multiple v-if + v-for', () => { - checkWarning( - ` - -
hey
-
hey
- - `, - true - ) - }) - - test('warns with template v-if', () => { - checkWarning( - ` - - - - `, - true - ) - }) - - test('warns with multiple templates', () => { - checkWarning( - ` - - - - - `, - true - ) - }) - - test('warns if multiple children with v-if', () => { - checkWarning( - ` - -
hey
-
hey
- - `, - true - ) - }) - - test('does not warn with regular element', () => { - checkWarning( - ` - -
hey
- - `, - false - ) - }) - - test('does not warn with one single v-if', () => { - checkWarning( - ` - -
hey
- - `, - false - ) - }) - - test('does not warn with v-if v-else-if v-else', () => { - checkWarning( - ` - -
hey
-
hey
-
hey
- - `, - false - ) - }) - - test('does not warn with v-if v-else', () => { - checkWarning( - ` - -
hey
-
hey
- - `, - false - ) - }) - }) -}) - -test('the v-if/else-if/else branches in Transition should ignore comments', () => { - expect( - compile(` - -
hey
- -
hey
- -
-

- -

-

- - `).code - ).toMatchSnapshot() -}) diff --git a/packages/compiler-dom/src/index.ts b/packages/compiler-dom/src/index.ts index 3687d84b..fc1fa040 100644 --- a/packages/compiler-dom/src/index.ts +++ b/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 = { - cloak: noopDirectiveTransform, +cloak: noopDirectiveTransform, html: transformVHtml, text: transformVText, model: transformModel, // override compiler-core diff --git a/packages/compiler-dom/src/transforms/warnTransitionChildren.ts b/packages/compiler-dom/src/transforms/Transition.ts similarity index 62% rename from packages/compiler-dom/src/transforms/warnTransitionChildren.ts rename to packages/compiler-dom/src/transforms/Transition.ts index 4e9bdba2..56d05ef0 100644 --- a/packages/compiler-dom/src/transforms/warnTransitionChildren.ts +++ b/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 + }) + } + } + } } } } diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts index bd4a03e8..dc014974 100644 --- a/packages/runtime-core/src/renderer.ts +++ b/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) } diff --git a/packages/vue/__tests__/Transition.spec.ts b/packages/vue/__tests__/Transition.spec.ts index 9ec7e6ca..61926310 100644 --- a/packages/vue/__tests__/Transition.spec.ts +++ b/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: `
@@ -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">
content
@@ -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('
content
') + 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: ` +
+ +
content
+ +
+ + `, + 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( + '
content
' + ) + expect(afterEnterSpy).toBeCalled() + }, + E2E_TIMEOUT + ) }) describe('explicit durations', () => { diff --git a/scripts/filter-e2e.js b/scripts/filter-e2e.js index bbc36022..db1d7c2c 100644 --- a/scripts/filter-e2e.js +++ b/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 {