fix(attr-fallthrough): ensure consistent attr fallthrough for root fragments with comments

fix #2549

packages/compiler-core/__tests__/transform.spec.ts

@@ -342,5 +342,24 @@ describe('compiler: transform', () => {
         )
       )
     })
+
+    test('multiple children w/ single root + comments', () => {
+      const ast = transformWithCodegen(`<!--foo--><div/><!--bar-->`)
+      expect(ast.codegenNode).toMatchObject(
+        createBlockMatcher(
+          FRAGMENT,
+          undefined,
+          [
+            { type: NodeTypes.COMMENT },
+            { type: NodeTypes.ELEMENT, tag: `div` },
+            { type: NodeTypes.COMMENT }
+          ] as any,
+          genFlagText([
+            PatchFlags.STABLE_FRAGMENT,
+            PatchFlags.DEV_ROOT_FRAGMENT
+          ])
+        )
+      )
+    })
   })
 })

packages/compiler-core/src/transform.ts

@@ -314,14 +314,23 @@ function createRootCodegen(root: RootNode, context: TransformContext) {
     }
   } else if (children.length > 1) {
     // root has multiple nodes - return a fragment block.
+    let patchFlag = PatchFlags.STABLE_FRAGMENT
+    let patchFlagText = PatchFlagNames[PatchFlags.STABLE_FRAGMENT]
+    // check if the fragment actually contains a single valid child with
+    // the rest being comments
+    if (
+      __DEV__ &&
+      children.filter(c => c.type !== NodeTypes.COMMENT).length === 1
+    ) {
+      patchFlag |= PatchFlags.DEV_ROOT_FRAGMENT
+      patchFlagText += `, ${PatchFlagNames[PatchFlags.DEV_ROOT_FRAGMENT]}`
+    }
     root.codegenNode = createVNodeCall(
       context,
       helper(FRAGMENT),
       undefined,
       root.children,
-      `${PatchFlags.STABLE_FRAGMENT} /* ${
-        PatchFlagNames[PatchFlags.STABLE_FRAGMENT]
-      } */`,
+      patchFlag + (__DEV__ ? ` /* ${patchFlagText} */` : ``),
       undefined,
       undefined,
       true

packages/runtime-core/__tests__/rendererAttrsFallthrough.spec.ts

@@ -13,6 +13,7 @@ import {
   createCommentVNode,
   Fragment
 } from '@vue/runtime-dom'
+import { PatchFlags } from '@vue/shared/src'
 
 describe('attribute fallthrough', () => {
   it('should allow attrs to fallthrough', async () => {
@@ -574,11 +575,16 @@ describe('attribute fallthrough', () => {
       setup() {
         return () => (
           openBlock(),
-          createBlock(Fragment, null, [
-            createCommentVNode('hello'),
-            h('button'),
-            createCommentVNode('world')
-          ])
+          createBlock(
+            Fragment,
+            null,
+            [
+              createCommentVNode('hello'),
+              h('button'),
+              createCommentVNode('world')
+            ],
+            PatchFlags.STABLE_FRAGMENT | PatchFlags.DEV_ROOT_FRAGMENT
+          )
         )
       }
     }

packages/runtime-core/src/componentRenderUtils.ts

@@ -9,7 +9,6 @@ import {
   createVNode,
   Comment,
   cloneVNode,
-  Fragment,
   VNodeArrayChildren,
   isVNode
 } from './vnode'
@@ -20,8 +19,10 @@ import { isHmrUpdating } from './hmr'
 import { NormalizedProps } from './componentProps'
 import { isEmitListener } from './componentEmits'
 
-// mark the current rendering instance for asset resolution (e.g.
-// resolveComponent, resolveDirective) during render
+/**
+ * mark the current rendering instance for asset resolution (e.g.
+ * resolveComponent, resolveDirective) during render
+ */
 export let currentRenderingInstance: ComponentInternalInstance | null = null
 
 export function setCurrentRenderingInstance(
@@ -30,9 +31,11 @@ export function setCurrentRenderingInstance(
   currentRenderingInstance = instance
 }
 
-// dev only flag to track whether $attrs was used during render.
-// If $attrs was used during render then the warning for failed attrs
-// fallthrough can be suppressed.
+/**
+ * dev only flag to track whether $attrs was used during render.
+ * If $attrs was used during render then the warning for failed attrs
+ * fallthrough can be suppressed.
+ */
 let accessedAttrs: boolean = false
 
 export function markAttrsAccessed() {
@@ -116,7 +119,7 @@ export function renderComponentRoot(
     // to have comments along side the root element which makes it a fragment
     let root = result
     let setRoot: ((root: VNode) => void) | undefined = undefined
-    if (__DEV__) {
+    if (__DEV__ && result.patchFlag & PatchFlags.DEV_ROOT_FRAGMENT) {
       ;[root, setRoot] = getChildRoot(result)
     }
 
@@ -222,9 +225,6 @@ export function renderComponentRoot(
 const getChildRoot = (
   vnode: VNode
 ): [VNode, ((root: VNode) => void) | undefined] => {
-  if (vnode.type !== Fragment) {
-    return [vnode, undefined]
-  }
   const rawChildren = vnode.children as VNodeArrayChildren
   const dynamicChildren = vnode.dynamicChildren
   const childRoot = filterSingleRoot(rawChildren)
@@ -246,18 +246,27 @@ const getChildRoot = (
   return [normalizeVNode(childRoot), setRoot]
 }
 
-/**
- * dev only
- */
-export function filterSingleRoot(children: VNodeArrayChildren): VNode | null {
-  const filtered = children.filter(child => {
-    return !(
-      isVNode(child) &&
-      child.type === Comment &&
-      child.children !== 'v-if'
-    )
-  })
-  return filtered.length === 1 && isVNode(filtered[0]) ? filtered[0] : null
+export function filterSingleRoot(
+  children: VNodeArrayChildren
+): VNode | undefined {
+  let singleRoot
+  for (let i = 0; i < children.length; i++) {
+    const child = children[i]
+    if (isVNode(child)) {
+      // ignore user comment
+      if (child.type !== Comment || child.children === 'v-if') {
+        if (singleRoot) {
+          // has more than 1 non-comment child, return now
+          return
+        } else {
+          singleRoot = child
+        }
+      }
+    } else {
+      return
+    }
+  }
+  return singleRoot
 }
 
 const getFunctionalFallthrough = (attrs: Data): Data | undefined => {

packages/shared/src/patchFlags.ts

@@ -1,89 +1,127 @@
-// Patch flags are optimization hints generated by the compiler.
-// when a block with dynamicChildren is encountered during diff, the algorithm
-// enters "optimized mode". In this mode, we know that the vdom is produced by
-// a render function generated by the compiler, so the algorithm only needs to
-// handle updates explicitly marked by these patch flags.
-
-// Patch flags can be combined using the | bitwise operator and can be checked
-// using the & operator, e.g.
-//
-//   const flag = TEXT | CLASS
-//   if (flag & TEXT) { ... }
-//
-// Check the `patchElement` function in './renderer.ts' to see how the
-// flags are handled during diff.
-
+/**
+ * Patch flags are optimization hints generated by the compiler.
+ * when a block with dynamicChildren is encountered during diff, the algorithm
+ * enters "optimized mode". In this mode, we know that the vdom is produced by
+ * a render function generated by the compiler, so the algorithm only needs to
+ * handle updates explicitly marked by these patch flags.
+ *
+ * Patch flags can be combined using the | bitwise operator and can be checked
+ * using the & operator, e.g.
+ *
+ * ```js
+ * const flag = TEXT | CLASS
+ * if (flag & TEXT) { ... }
+ * ```
+ *
+ * Check the `patchElement` function in './renderer.ts' to see how the
+ * flags are handled during diff.
+ */
 export const enum PatchFlags {
-  // Indicates an element with dynamic textContent (children fast path)
+  /**
+   * Indicates an element with dynamic textContent (children fast path)
+   */
   TEXT = 1,
 
-  // Indicates an element with dynamic class binding.
+  /**
+   * Indicates an element with dynamic class binding.
+   */
   CLASS = 1 << 1,
 
-  // Indicates an element with dynamic style
-  // The compiler pre-compiles static string styles into static objects
-  // + detects and hoists inline static objects
-  // e.g. style="color: red" and :style="{ color: 'red' }" both get hoisted as
-  //   const style = { color: 'red' }
-  //   render() { return e('div', { style }) }
+  /**
+   * Indicates an element with dynamic style
+   * The compiler pre-compiles static string styles into static objects
+   * + detects and hoists inline static objects
+   * e.g. style="color: red" and :style="{ color: 'red' }" both get hoisted as
+   *   const style = { color: 'red' }
+   *   render() { return e('div', { style }) }
+   */
   STYLE = 1 << 2,
 
-  // Indicates an element that has non-class/style dynamic props.
-  // Can also be on a component that has any dynamic props (includes
-  // class/style). when this flag is present, the vnode also has a dynamicProps
-  // array that contains the keys of the props that may change so the runtime
-  // can diff them faster (without having to worry about removed props)
+  /**
+   * Indicates an element that has non-class/style dynamic props.
+   * Can also be on a component that has any dynamic props (includes
+   * class/style). when this flag is present, the vnode also has a dynamicProps
+   * array that contains the keys of the props that may change so the runtime
+   * can diff them faster (without having to worry about removed props)
+   */
   PROPS = 1 << 3,
 
-  // Indicates an element with props with dynamic keys. When keys change, a full
-  // diff is always needed to remove the old key. This flag is mutually
-  // exclusive with CLASS, STYLE and PROPS.
+  /**
+   * Indicates an element with props with dynamic keys. When keys change, a full
+   * diff is always needed to remove the old key. This flag is mutually
+   * exclusive with CLASS, STYLE and PROPS.
+   */
   FULL_PROPS = 1 << 4,
 
-  // Indicates an element with event listeners (which need to be attached
-  // during hydration)
+  /**
+   * Indicates an element with event listeners (which need to be attached
+   * during hydration)
+   */
   HYDRATE_EVENTS = 1 << 5,
 
-  // Indicates a fragment whose children order doesn't change.
+  /**
+   * Indicates a fragment whose children order doesn't change.
+   */
   STABLE_FRAGMENT = 1 << 6,
 
-  // Indicates a fragment with keyed or partially keyed children
+  /**
+   * Indicates a fragment with keyed or partially keyed children
+   */
   KEYED_FRAGMENT = 1 << 7,
 
-  // Indicates a fragment with unkeyed children.
+  /**
+   * Indicates a fragment with unkeyed children.
+   */
   UNKEYED_FRAGMENT = 1 << 8,
 
-  // Indicates an element that only needs non-props patching, e.g. ref or
-  // directives (onVnodeXXX hooks). since every patched vnode checks for refs
-  // and onVnodeXXX hooks, it simply marks the vnode so that a parent block
-  // will track it.
+  /**
+   * Indicates an element that only needs non-props patching, e.g. ref or
+   * directives (onVnodeXXX hooks). since every patched vnode checks for refs
+   * and onVnodeXXX hooks, it simply marks the vnode so that a parent block
+   * will track it.
+   */
   NEED_PATCH = 1 << 9,
 
-  // Indicates a component with dynamic slots (e.g. slot that references a v-for
-  // iterated value, or dynamic slot names).
-  // Components with this flag are always force updated.
+  /**
+   * Indicates a component with dynamic slots (e.g. slot that references a v-for
+   * iterated value, or dynamic slot names).
+   * Components with this flag are always force updated.
+   */
   DYNAMIC_SLOTS = 1 << 10,
 
-  // SPECIAL FLAGS -------------------------------------------------------------
-
-  // Special flags are negative integers. They are never matched against using
-  // bitwise operators (bitwise matching should only happen in branches where
-  // patchFlag > 0), and are mutually exclusive. When checking for a special
-  // flag, simply check patchFlag === FLAG.
-
-  // Indicates a hoisted static vnode. This is a hint for hydration to skip
-  // the entire sub tree since static content never needs to be updated.
+  /**
+   * Indicates a fragment that was created only because the user has placed
+   * comments at the root level of a template. This is a dev-only flag since
+   * comments are stripped in production.
+   */
+  DEV_ROOT_FRAGMENT = 1 << 11,
+
+  /**
+   * SPECIAL FLAGS -------------------------------------------------------------
+   * Special flags are negative integers. They are never matched against using
+   * bitwise operators (bitwise matching should only happen in branches where
+   * patchFlag > 0), and are mutually exclusive. When checking for a special
+   * flag, simply check patchFlag === FLAG.
+   */
+
+  /**
+   * Indicates a hoisted static vnode. This is a hint for hydration to skip
+   * the entire sub tree since static content never needs to be updated.
+   */
   HOISTED = -1,
-
-  // A special flag that indicates that the diffing algorithm should bail out
-  // of optimized mode. For example, on block fragments created by renderSlot()
-  // when encountering non-compiler generated slots (i.e. manually written
-  // render functions, which should always be fully diffed)
-  // OR manually cloneVNodes
+  /**
+   * A special flag that indicates that the diffing algorithm should bail out
+   * of optimized mode. For example, on block fragments created by renderSlot()
+   * when encountering non-compiler generated slots (i.e. manually written
+   * render functions, which should always be fully diffed)
+   * OR manually cloneVNodes
+   */
   BAIL = -2
 }
 
-// dev only flag -> name mapping
+/**
+ * dev only flag -> name mapping
+ */
 export const PatchFlagNames = {
   [PatchFlags.TEXT]: `TEXT`,
   [PatchFlags.CLASS]: `CLASS`,
@@ -94,8 +132,9 @@ export const PatchFlagNames = {
   [PatchFlags.STABLE_FRAGMENT]: `STABLE_FRAGMENT`,
   [PatchFlags.KEYED_FRAGMENT]: `KEYED_FRAGMENT`,
   [PatchFlags.UNKEYED_FRAGMENT]: `UNKEYED_FRAGMENT`,
-  [PatchFlags.DYNAMIC_SLOTS]: `DYNAMIC_SLOTS`,
   [PatchFlags.NEED_PATCH]: `NEED_PATCH`,
+  [PatchFlags.DYNAMIC_SLOTS]: `DYNAMIC_SLOTS`,
+  [PatchFlags.DEV_ROOT_FRAGMENT]: `DEV_ROOT_FRAGMENT`,
   [PatchFlags.HOISTED]: `HOISTED`,
   [PatchFlags.BAIL]: `BAIL`
 }