From f4f5b4fd29c60fc0c5a0f2bc5107bedab1fa86b0 Mon Sep 17 00:00:00 2001 From: Andrew Talbot Date: Mon, 20 Apr 2020 16:40:59 -0400 Subject: [PATCH] feat(runtime-core): improve warning for extraneous event listeners (#1005) fix #1001 --- .../rendererAttrsFallthrough.spec.ts | 72 ++++++++++++++++++- packages/runtime-core/src/component.ts | 4 +- .../runtime-core/src/componentRenderUtils.ts | 63 +++++++++++----- 3 files changed, 119 insertions(+), 20 deletions(-) diff --git a/packages/runtime-core/__tests__/rendererAttrsFallthrough.spec.ts b/packages/runtime-core/__tests__/rendererAttrsFallthrough.spec.ts index 4f9d208f8dc..d8452db58f9 100644 --- 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(`
`) }) 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(`
`) }) + 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(`
`) + }) + + 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(`
`) + }) + + 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(`
`) + }) + // #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 index 6ba7c0ffa0f..a2387a648a1 100644 --- a/packages/runtime-core/src/component.ts +++ b/packages/runtime-core/src/component.ts @@ -490,7 +490,9 @@ function finishComponentSetup( const attrHandlers: ProxyHandler = { 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 index 3a9f8d888c7..4fd61895fa1 100644 --- 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.` + ) + } } }