fix: vapor transition multiple chilren check

Author: edison1105

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

@@ -24,6 +24,9 @@ export const transformTransition: NodeTransform = (node, context) => {
 export function postTransformTransition(
   node: ComponentNode,
   onError: (error: CompilerError) => void,
+  hasMultipleChildren: (
+    node: ComponentNode,
+  ) => boolean = defaultHasMultipleChildren,
 ): () => void {
   return () => {
     if (!node.children.length) {
@@ -59,7 +62,9 @@ export function postTransformTransition(
   }
 }
 
-function hasMultipleChildren(node: ComponentNode | IfBranchNode): boolean {
+function defaultHasMultipleChildren(
+  node: ComponentNode | IfBranchNode,
+): boolean {
   // #1352 filter out potential comment nodes.
   const children = (node.children = node.children.filter(
     c =>
@@ -70,6 +75,7 @@ function hasMultipleChildren(node: ComponentNode | IfBranchNode): boolean {
   return (
     children.length !== 1 ||
     child.type === NodeTypes.FOR ||
-    (child.type === NodeTypes.IF && child.branches.some(hasMultipleChildren))
+    (child.type === NodeTypes.IF &&
+      child.branches.some(defaultHasMultipleChildren))
   )
 }

packages/compiler-vapor/__tests__/transforms/TransformTransition.spec.ts

@@ -57,21 +57,132 @@ describe('compiler: transition', () => {
     expect(code).contains('n0.$key = _ctx.key')
   })
 
-  test('warns if multiple children', () => {
+  function checkWarning(template: string, shouldWarn = true) {
     const onError = vi.fn()
-    compileWithElementTransform(
+    compileWithElementTransform(template, { onError })
+    if (shouldWarn) {
+      expect(onError).toHaveBeenCalled()
+      expect(onError.mock.calls).toMatchObject([
+        [{ code: DOMErrorCodes.X_TRANSITION_INVALID_CHILDREN }],
+      ])
+    } else {
+      expect(onError).not.toHaveBeenCalled()
+    }
+  }
+
+  test('warns if multiple children', () => {
+    checkWarning(
       `<Transition>
         <h1>foo</h1>
         <h2>bar</h2>
       </Transition>`,
-      {
-        onError,
-      },
+      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,
     )
-    expect(onError).toHaveBeenCalled()
-    expect(onError.mock.calls).toMatchObject([
-      [{ code: DOMErrorCodes.X_TRANSITION_INVALID_CHILDREN }],
-    ])
   })
 
   test('inject persisted when child has v-show', () => {
@@ -84,5 +195,21 @@ describe('compiler: transition', () => {
     ).toMatchSnapshot()
   })
 
-  // TODO more tests
+  test('the v-if/else-if/else branches in Transition should ignore comments', () => {
+    expect(
+      compileWithElementTransform(`
+    <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-vapor/__tests__/transforms/__snapshots__/TransformTransition.spec.ts.snap

@@ -35,6 +35,43 @@ export function render(_ctx) {
 }"
 `;
 
+exports[`compiler: transition > the v-if/else-if/else branches in Transition should ignore comments 1`] = `
+"import { VaporTransition as _VaporTransition, createIf as _createIf, prepend as _prepend, createComponent as _createComponent, template as _template } from 'vue';
+const t0 = _template("<div>hey</div>")
+const t1 = _template("<p></p>")
+const t2 = _template("<div></div>")
+
+export function render(_ctx) {
+  const n16 = _createComponent(_VaporTransition, null, {
+    "default": () => {
+      const n0 = _createIf(() => (_ctx.a), () => {
+        const n2 = t0()
+        n2.$key = 2
+        return n2
+      }, () => _createIf(() => (_ctx.b), () => {
+        const n5 = t0()
+        n5.$key = 5
+        return n5
+      }, () => {
+        const n14 = t2()
+        const n9 = _createIf(() => (_ctx.c), () => {
+          const n11 = t1()
+          return n11
+        }, () => {
+          const n13 = t1()
+          return n13
+        })
+        _prepend(n14, n9)
+        n14.$key = 14
+        return n14
+      }))
+      return [n0, n3, n7]
+    }
+  }, true)
+  return n16
+}"
+`;
+
 exports[`compiler: transition > work with dynamic keyed children 1`] = `
 "import { VaporTransition as _VaporTransition, createKeyedFragment as _createKeyedFragment, createComponent as _createComponent, template as _template } from 'vue';
 const t0 = _template("<h1>foo</h1>")

packages/compiler-vapor/src/transforms/transformTransition.ts

@@ -1,15 +1,69 @@
 import type { NodeTransform } from '@vue/compiler-vapor'
-import { ElementTypes, NodeTypes } from '@vue/compiler-core'
-import { isTransitionTag } from '../utils'
-import { postTransformTransition } from '@vue/compiler-dom'
+import { findDir, isTransitionTag } from '../utils'
+import {
+  type ElementNode,
+  ElementTypes,
+  NodeTypes,
+  isTemplateNode,
+  postTransformTransition,
+} from '@vue/compiler-dom'
 
 export const transformTransition: NodeTransform = (node, context) => {
   if (
     node.type === NodeTypes.ELEMENT &&
     node.tagType === ElementTypes.COMPONENT
   ) {
     if (isTransitionTag(node.tag)) {
-      return postTransformTransition(node, context.options.onError)
+      return postTransformTransition(
+        node,
+        context.options.onError,
+        hasMultipleChildren,
+      )
     }
   }
 }
+
+function hasMultipleChildren(node: ElementNode): boolean {
+  const children = (node.children = node.children.filter(
+    c =>
+      c.type !== NodeTypes.COMMENT &&
+      !(c.type === NodeTypes.TEXT && !c.content.trim()),
+  ))
+
+  const first = children[0]
+
+  // template
+  if (first && isTemplateNode(first)) {
+    return true
+  }
+
+  // has v-for
+  if (
+    children.length === 1 &&
+    first.type === NodeTypes.ELEMENT &&
+    findDir(first, 'for')
+  ) {
+    return true
+  }
+
+  const hasElse = (node: ElementNode) =>
+    findDir(node, 'else-if') || findDir(node, 'else', true)
+
+  // has v-if/v-else-if/v-else
+  if (
+    children.length > 1 &&
+    children.every(
+      (c, index) =>
+        c.type === NodeTypes.ELEMENT &&
+        // not has v-for
+        !findDir(c, 'for') &&
+        // if the first child has v-if, the rest should also have v-else-if/v-else
+        (index === 0 ? findDir(c, 'if') : hasElse(c)) &&
+        !hasMultipleChildren(c),
+    )
+  ) {
+    return false
+  }
+
+  return children.length > 1
+}