fix(attr-fallthrough): ensure consistent attr fallthrough for root fr…

…agments 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`
 }