feat(runtime-core): improve warning for extraneous event listeners (v…

…uejs#1005) fix vuejs#1001
pikax · Apr 29, 2020 · f4f5b4f · f4f5b4f
1 parent f8a9799
commit f4f5b4f
Show file tree

Hide file tree

Showing 3 changed files with 119 additions and 20 deletions.
diff --git a/packages/runtime-core/__tests__/rendererAttrsFallthrough.spec.ts b/packages/runtime-core/__tests__/rendererAttrsFallthrough.spec.ts
@@ -337,7 +337,7 @@ describe('attribute fallthrough', () => {
   it('should warn when fallthrough fails on non-single-root', () => {
     const Parent = {
       render() {
-        return h(Child, { foo: 1, class: 'parent' })
+        return h(Child, { foo: 1, class: 'parent', onBar: () => {} })
       }
     }
 
@@ -353,12 +353,13 @@ describe('attribute fallthrough', () => {
     render(h(Parent), root)
 
     expect(`Extraneous non-props attributes (class)`).toHaveBeenWarned()
+    expect(`Extraneous non-emits event listeners`).toHaveBeenWarned()
   })
 
   it('should not warn when $attrs is used during render', () => {
     const Parent = {
       render() {
-        return h(Child, { foo: 1, class: 'parent' })
+        return h(Child, { foo: 1, class: 'parent', onBar: () => {} })
       }
     }
 
@@ -374,13 +375,15 @@ describe('attribute fallthrough', () => {
     render(h(Parent), root)
 
     expect(`Extraneous non-props attributes`).not.toHaveBeenWarned()
+    expect(`Extraneous non-emits event listeners`).not.toHaveBeenWarned()
+
     expect(root.innerHTML).toBe(`<div></div><div class="parent"></div>`)
   })
 
   it('should not warn when context.attrs is used during render', () => {
     const Parent = {
       render() {
-        return h(Child, { foo: 1, class: 'parent' })
+        return h(Child, { foo: 1, class: 'parent', onBar: () => {} })
       }
     }
 
@@ -396,9 +399,72 @@ describe('attribute fallthrough', () => {
     render(h(Parent), root)
 
     expect(`Extraneous non-props attributes`).not.toHaveBeenWarned()
+    expect(`Extraneous non-emits event listeners`).not.toHaveBeenWarned()
+
     expect(root.innerHTML).toBe(`<div></div><div class="parent"></div>`)
   })
 
+  it('should not warn when context.attrs is used during render (functional)', () => {
+    const Parent = {
+      render() {
+        return h(Child, { foo: 1, class: 'parent', onBar: () => {} })
+      }
+    }
+
+    const Child: FunctionalComponent = (_, { attrs }) => [
+      h('div'),
+      h('div', attrs)
+    ]
+
+    Child.props = ['foo']
+
+    const root = document.createElement('div')
+    document.body.appendChild(root)
+    render(h(Parent), root)
+
+    expect(`Extraneous non-props attributes`).not.toHaveBeenWarned()
+    expect(`Extraneous non-emits event listeners`).not.toHaveBeenWarned()
+    expect(root.innerHTML).toBe(`<div></div><div class="parent"></div>`)
+  })
+
+  it('should not warn when functional component has optional props', () => {
+    const Parent = {
+      render() {
+        return h(Child, { foo: 1, class: 'parent', onBar: () => {} })
+      }
+    }
+
+    const Child = (props: any) => [h('div'), h('div', { class: props.class })]
+
+    const root = document.createElement('div')
+    document.body.appendChild(root)
+    render(h(Parent), root)
+
+    expect(`Extraneous non-props attributes`).not.toHaveBeenWarned()
+    expect(`Extraneous non-emits event listeners`).not.toHaveBeenWarned()
+    expect(root.innerHTML).toBe(`<div></div><div class="parent"></div>`)
+  })
+
+  it('should warn when functional component has props and does not use attrs', () => {
+    const Parent = {
+      render() {
+        return h(Child, { foo: 1, class: 'parent', onBar: () => {} })
+      }
+    }
+
+    const Child: FunctionalComponent = () => [h('div'), h('div')]
+
+    Child.props = ['foo']
+
+    const root = document.createElement('div')
+    document.body.appendChild(root)
+    render(h(Parent), root)
+
+    expect(`Extraneous non-props attributes`).toHaveBeenWarned()
+    expect(`Extraneous non-emits event listeners`).toHaveBeenWarned()
+    expect(root.innerHTML).toBe(`<div></div><div></div>`)
+  })
+
   // #677
   it('should update merged dynamic attrs on optimized child root', async () => {
     const aria = ref('true')

diff --git a/packages/runtime-core/src/component.ts b/packages/runtime-core/src/component.ts
@@ -490,7 +490,9 @@ function finishComponentSetup(
 
 const attrHandlers: ProxyHandler<Data> = {
   get: (target, key: string) => {
-    markAttrsAccessed()
+    if (__DEV__) {
+      markAttrsAccessed()
+    }
     return target[key]
   },
   set: () => {

diff --git a/packages/runtime-core/src/componentRenderUtils.ts b/packages/runtime-core/src/componentRenderUtils.ts
@@ -70,13 +70,25 @@ export function renderComponentRoot(
     } else {
       // functional
       const render = Component as FunctionalComponent
+      // in dev, mark attrs accessed if optional props (attrs === props)
+      if (__DEV__ && attrs === props) {
+        markAttrsAccessed()
+      }
       result = normalizeVNode(
         render.length > 1
-          ? render(props, {
-              attrs,
-              slots,
-              emit
-            })
+          ? render(
+              props,
+              __DEV__
+                ? {
+                    get attrs() {
+                      markAttrsAccessed()
+                      return attrs
+                    },
+                    slots,
+                    emit
+                  }
+                : { attrs, slots, emit }
+            )
           : render(props, null as any /* we know it doesn't need it */)
       )
       fallthroughAttrs = Component.props ? attrs : getFallthroughAttrs(attrs)
@@ -107,17 +119,36 @@ export function renderComponentRoot(
           root.patchFlag |= PatchFlags.FULL_PROPS
         }
       } else if (__DEV__ && !accessedAttrs && root.type !== Comment) {
-        const hasListeners = Object.keys(attrs).some(isOn)
-        warn(
-          `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.` +
-            (hasListeners
-              ? ` If the v-on listener is intended to be a component custom ` +
-                `event listener only, declare it using the "emits" option.`
-              : ``)
-        )
+        const allAttrs = Object.keys(attrs)
+        const eventAttrs: string[] = []
+        const extraAttrs: string[] = []
+        for (let i = 0, l = allAttrs.length; i < l; i++) {
+          const key = allAttrs[i]
+          if (isOn(key)) {
+            // remove `on`, lowercase first letter to reflect event casing accurately
+            eventAttrs.push(key[2].toLowerCase() + key.slice(3))
+          } else {
+            extraAttrs.push(key)
+          }
+        }
+        if (extraAttrs.length) {
+          warn(
+            `Extraneous non-props attributes (` +
+              `${extraAttrs.join(', ')}) ` +
+              `were passed to component but could not be automatically inherited ` +
+              `because component renders fragment or text root nodes.`
+          )
+        }
+        if (eventAttrs.length) {
+          warn(
+            `Extraneous non-emits event listeners (` +
+              `${eventAttrs.join(', ')}) ` +
+              `were passed to component but could not be automatically inherited ` +
+              `because component renders fragment or text root nodes. ` +
+              `If the listener is intended to be a component custom event listener only, ` +
+              `declare it using the "emits" option.`
+          )
+        }
       }
     }