refactor(runtime-core): adjust attr fallthrough behavior

BREAKING CHANGE: adjust attr fallthrough behavior

    Updated per pending RFC vuejs/rfcs#137

    - Implicit fallthrough now by default only applies for a whitelist
      of attributes (class, style, event listeners, a11y attributes, and
      data attributes).

    - Fallthrough is now applied regardless of whether the component has
      explicitly declared props. (close #749)

Author: yyx990803

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

@@ -15,7 +15,7 @@ import { mockWarn } from '@vue/shared'
 describe('attribute fallthrough', () => {
   mockWarn()
 
-  it('everything should be in props when component has no declared props', async () => {
+  it('should allow whitelisted attrs to fallthrough', async () => {
     const click = jest.fn()
     const childUpdated = jest.fn()
 
@@ -42,113 +42,42 @@ describe('attribute fallthrough', () => {
 
     const Child = {
       setup(props: any) {
-        onUpdated(childUpdated)
-        return () =>
-          h(
-            'div',
-            mergeProps(
-              {
-                class: 'c2',
-                style: { fontWeight: 'bold' }
-              },
-              props
-            ),
-            props.foo
-          )
-      }
-    }
-
-    const root = document.createElement('div')
-    document.body.appendChild(root)
-    render(h(Hello), root)
-
-    const node = root.children[0] as HTMLElement
-
-    expect(node.getAttribute('id')).toBe('test')
-    expect(node.getAttribute('foo')).toBe('1')
-    expect(node.getAttribute('class')).toBe('c2 c0')
-    expect(node.style.color).toBe('green')
-    expect(node.style.fontWeight).toBe('bold')
-    expect(node.dataset.id).toBe('1')
-    node.dispatchEvent(new CustomEvent('click'))
-    expect(click).toHaveBeenCalled()
-
-    await nextTick()
-    expect(childUpdated).toHaveBeenCalled()
-    expect(node.getAttribute('id')).toBe('test')
-    expect(node.getAttribute('foo')).toBe('1')
-    expect(node.getAttribute('class')).toBe('c2 c1')
-    expect(node.style.color).toBe('red')
-    expect(node.style.fontWeight).toBe('bold')
-  })
-
-  it('should implicitly fallthrough on single root nodes', async () => {
-    const click = jest.fn()
-    const childUpdated = jest.fn()
-
-    const Hello = {
-      setup() {
-        const count = ref(0)
-
-        function inc() {
-          count.value++
-          click()
-        }
-
-        return () =>
-          h(Child, {
-            foo: 1,
-            id: 'test',
-            class: 'c' + count.value,
-            style: { color: count.value ? 'red' : 'green' },
-            onClick: inc
-          })
-      }
-    }
-
-    const Child = defineComponent({
-      props: {
-        foo: Number
-      },
-      setup(props) {
         onUpdated(childUpdated)
         return () =>
           h(
             'div',
             {
+              id: props.id, // id is not whitelisted
               class: 'c2',
               style: { fontWeight: 'bold' }
             },
             props.foo
           )
       }
-    })
+    }
 
     const root = document.createElement('div')
     document.body.appendChild(root)
     render(h(Hello), root)
 
     const node = root.children[0] as HTMLElement
 
-    // with declared props, any parent attr that isn't a prop falls through
-    expect(node.getAttribute('id')).toBe('test')
+    expect(node.getAttribute('id')).toBe('test') // id is not whitelisted, but explicitly bound
+    expect(node.getAttribute('foo')).toBe(null) // foo is not whitelisted
     expect(node.getAttribute('class')).toBe('c2 c0')
     expect(node.style.color).toBe('green')
     expect(node.style.fontWeight).toBe('bold')
+    expect(node.dataset.id).toBe('1')
     node.dispatchEvent(new CustomEvent('click'))
     expect(click).toHaveBeenCalled()
 
-    // ...while declared ones remain props
-    expect(node.hasAttribute('foo')).toBe(false)
-
     await nextTick()
     expect(childUpdated).toHaveBeenCalled()
     expect(node.getAttribute('id')).toBe('test')
+    expect(node.getAttribute('foo')).toBe(null)
     expect(node.getAttribute('class')).toBe('c2 c1')
     expect(node.style.color).toBe('red')
     expect(node.style.fontWeight).toBe('bold')
-
-    expect(node.hasAttribute('foo')).toBe(false)
   })
 
   it('should fallthrough for nested components', async () => {
@@ -179,12 +108,16 @@ describe('attribute fallthrough', () => {
     const Child = {
       setup(props: any) {
         onUpdated(childUpdated)
+        // HOC simply passing props down.
+        // this will result in merging the same attrs, but should be deduped by
+        // `mergeProps`.
         return () => h(GrandChild, props)
       }
     }
 
     const GrandChild = defineComponent({
       props: {
+        id: String,
         foo: Number
       },
       setup(props) {
@@ -193,6 +126,7 @@ describe('attribute fallthrough', () => {
           h(
             'div',
             {
+              id: props.id,
               class: 'c2',
               style: { fontWeight: 'bold' }
             },
@@ -351,11 +285,11 @@ describe('attribute fallthrough', () => {
 
   // #677
   it('should update merged dynamic attrs on optimized child root', async () => {
-    const id = ref('foo')
+    const aria = ref('true')
     const cls = ref('bar')
     const Parent = {
       render() {
-        return h(Child, { id: id.value, class: cls.value })
+        return h(Child, { 'aria-hidden': aria.value, class: cls.value })
       }
     }
 
@@ -370,14 +304,14 @@ describe('attribute fallthrough', () => {
     document.body.appendChild(root)
     render(h(Parent), root)
 
-    expect(root.innerHTML).toBe(`<div id="foo" class="bar"></div>`)
+    expect(root.innerHTML).toBe(`<div aria-hidden="true" class="bar"></div>`)
 
-    id.value = 'fooo'
+    aria.value = 'false'
     await nextTick()
-    expect(root.innerHTML).toBe(`<div id="fooo" class="bar"></div>`)
+    expect(root.innerHTML).toBe(`<div aria-hidden="false" class="bar"></div>`)
 
     cls.value = 'barr'
     await nextTick()
-    expect(root.innerHTML).toBe(`<div id="fooo" class="barr"></div>`)
+    expect(root.innerHTML).toBe(`<div aria-hidden="false" class="barr"></div>`)
   })
 })

packages/runtime-core/src/componentProps.ts

@@ -214,7 +214,7 @@ export function resolveProps(
   lock()
 
   instance.props = props
-  instance.attrs = options ? attrs || EMPTY_OBJ : props
+  instance.attrs = attrs || EMPTY_OBJ
   instance.vnodeHooks = vnodeHooks || EMPTY_OBJ
 }
 

packages/runtime-core/src/componentRenderUtils.ts

@@ -11,7 +11,7 @@ import {
   cloneVNode
 } from './vnode'
 import { handleError, ErrorCodes } from './errorHandling'
-import { PatchFlags, ShapeFlags, EMPTY_OBJ } from '@vue/shared'
+import { PatchFlags, ShapeFlags, EMPTY_OBJ, isOn } from '@vue/shared'
 import { warn } from './warning'
 
 // mark the current rendering instance for asset resolution (e.g.
@@ -79,25 +79,26 @@ export function renderComponentRoot(
     }
 
     // attr merging
+    let fallthroughAttrs
     if (
-      Component.props != null &&
       Component.inheritAttrs !== false &&
       attrs !== EMPTY_OBJ &&
-      Object.keys(attrs).length
+      (fallthroughAttrs = getFallthroughAttrs(attrs))
     ) {
       if (
         result.shapeFlag & ShapeFlags.ELEMENT ||
         result.shapeFlag & ShapeFlags.COMPONENT
       ) {
-        result = cloneVNode(result, attrs)
+        result = cloneVNode(result, fallthroughAttrs)
         // If the child root node is a compiler optimized vnode, make sure it
         // force update full props to account for the merged attrs.
         if (result.dynamicChildren !== null) {
           result.patchFlag |= PatchFlags.FULL_PROPS
         }
       } else if (__DEV__ && !accessedAttrs && result.type !== Comment) {
         warn(
-          `Extraneous non-props attributes (${Object.keys(attrs).join(',')}) ` +
+          `Extraneous non-props attributes (` +
+            `${Object.keys(attrs).join(', ')}) ` +
             `were passed to component but could not be automatically inherited ` +
             `because component renders fragment or text root nodes.`
         )
@@ -142,7 +143,24 @@ export function renderComponentRoot(
   return result
 }
 
-function isElementRoot(vnode: VNode) {
+const getFallthroughAttrs = (attrs: Data): Data | undefined => {
+  let res: Data | undefined
+  for (const key in attrs) {
+    if (
+      key === 'class' ||
+      key === 'style' ||
+      key === 'role' ||
+      isOn(key) ||
+      key.indexOf('aria-') === 0 ||
+      key.indexOf('data-') === 0
+    ) {
+      ;(res || (res = {}))[key] = attrs[key]
+    }
+  }
+  return res
+}
+
+const isElementRoot = (vnode: VNode) => {
   return (
     vnode.shapeFlag & ShapeFlags.COMPONENT ||
     vnode.shapeFlag & ShapeFlags.ELEMENT ||

packages/runtime-core/src/vnode.ts

@@ -399,15 +399,20 @@ export function mergeProps(...args: (Data & VNodeProps)[]) {
     const toMerge = args[i]
     for (const key in toMerge) {
       if (key === 'class') {
-        ret.class = normalizeClass([ret.class, toMerge.class])
+        if (ret.class !== toMerge.class) {
+          ret.class = normalizeClass([ret.class, toMerge.class])
+        }
       } else if (key === 'style') {
         ret.style = normalizeStyle([ret.style, toMerge.style])
       } else if (handlersRE.test(key)) {
         // on*, vnode*
         const existing = ret[key]
-        ret[key] = existing
-          ? [].concat(existing as any, toMerge[key] as any)
-          : toMerge[key]
+        const incoming = toMerge[key]
+        if (existing !== incoming) {
+          ret[key] = existing
+            ? [].concat(existing as any, toMerge[key] as any)
+            : incoming
+        }
       } else {
         ret[key] = toMerge[key]
       }