Skip to content

Commit cebad64

Browse files
author
Andrew Talbot
authored
feat(runtime-core): improve warning for extraneous event listeners (#1005)
fix #1001
1 parent dece610 commit cebad64

File tree

3 files changed

+119
-20
lines changed

3 files changed

+119
-20
lines changed

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

+69-3
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ describe('attribute fallthrough', () => {
337337
it('should warn when fallthrough fails on non-single-root', () => {
338338
const Parent = {
339339
render() {
340-
return h(Child, { foo: 1, class: 'parent' })
340+
return h(Child, { foo: 1, class: 'parent', onBar: () => {} })
341341
}
342342
}
343343

@@ -353,12 +353,13 @@ describe('attribute fallthrough', () => {
353353
render(h(Parent), root)
354354

355355
expect(`Extraneous non-props attributes (class)`).toHaveBeenWarned()
356+
expect(`Extraneous non-emits event listeners`).toHaveBeenWarned()
356357
})
357358

358359
it('should not warn when $attrs is used during render', () => {
359360
const Parent = {
360361
render() {
361-
return h(Child, { foo: 1, class: 'parent' })
362+
return h(Child, { foo: 1, class: 'parent', onBar: () => {} })
362363
}
363364
}
364365

@@ -374,13 +375,15 @@ describe('attribute fallthrough', () => {
374375
render(h(Parent), root)
375376

376377
expect(`Extraneous non-props attributes`).not.toHaveBeenWarned()
378+
expect(`Extraneous non-emits event listeners`).not.toHaveBeenWarned()
379+
377380
expect(root.innerHTML).toBe(`<div></div><div class="parent"></div>`)
378381
})
379382

380383
it('should not warn when context.attrs is used during render', () => {
381384
const Parent = {
382385
render() {
383-
return h(Child, { foo: 1, class: 'parent' })
386+
return h(Child, { foo: 1, class: 'parent', onBar: () => {} })
384387
}
385388
}
386389

@@ -396,9 +399,72 @@ describe('attribute fallthrough', () => {
396399
render(h(Parent), root)
397400

398401
expect(`Extraneous non-props attributes`).not.toHaveBeenWarned()
402+
expect(`Extraneous non-emits event listeners`).not.toHaveBeenWarned()
403+
399404
expect(root.innerHTML).toBe(`<div></div><div class="parent"></div>`)
400405
})
401406

407+
it('should not warn when context.attrs is used during render (functional)', () => {
408+
const Parent = {
409+
render() {
410+
return h(Child, { foo: 1, class: 'parent', onBar: () => {} })
411+
}
412+
}
413+
414+
const Child: FunctionalComponent = (_, { attrs }) => [
415+
h('div'),
416+
h('div', attrs)
417+
]
418+
419+
Child.props = ['foo']
420+
421+
const root = document.createElement('div')
422+
document.body.appendChild(root)
423+
render(h(Parent), root)
424+
425+
expect(`Extraneous non-props attributes`).not.toHaveBeenWarned()
426+
expect(`Extraneous non-emits event listeners`).not.toHaveBeenWarned()
427+
expect(root.innerHTML).toBe(`<div></div><div class="parent"></div>`)
428+
})
429+
430+
it('should not warn when functional component has optional props', () => {
431+
const Parent = {
432+
render() {
433+
return h(Child, { foo: 1, class: 'parent', onBar: () => {} })
434+
}
435+
}
436+
437+
const Child = (props: any) => [h('div'), h('div', { class: props.class })]
438+
439+
const root = document.createElement('div')
440+
document.body.appendChild(root)
441+
render(h(Parent), root)
442+
443+
expect(`Extraneous non-props attributes`).not.toHaveBeenWarned()
444+
expect(`Extraneous non-emits event listeners`).not.toHaveBeenWarned()
445+
expect(root.innerHTML).toBe(`<div></div><div class="parent"></div>`)
446+
})
447+
448+
it('should warn when functional component has props and does not use attrs', () => {
449+
const Parent = {
450+
render() {
451+
return h(Child, { foo: 1, class: 'parent', onBar: () => {} })
452+
}
453+
}
454+
455+
const Child: FunctionalComponent = () => [h('div'), h('div')]
456+
457+
Child.props = ['foo']
458+
459+
const root = document.createElement('div')
460+
document.body.appendChild(root)
461+
render(h(Parent), root)
462+
463+
expect(`Extraneous non-props attributes`).toHaveBeenWarned()
464+
expect(`Extraneous non-emits event listeners`).toHaveBeenWarned()
465+
expect(root.innerHTML).toBe(`<div></div><div></div>`)
466+
})
467+
402468
// #677
403469
it('should update merged dynamic attrs on optimized child root', async () => {
404470
const aria = ref('true')

packages/runtime-core/src/component.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,9 @@ function finishComponentSetup(
490490

491491
const attrHandlers: ProxyHandler<Data> = {
492492
get: (target, key: string) => {
493-
markAttrsAccessed()
493+
if (__DEV__) {
494+
markAttrsAccessed()
495+
}
494496
return target[key]
495497
},
496498
set: () => {

packages/runtime-core/src/componentRenderUtils.ts

+47-16
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,25 @@ export function renderComponentRoot(
7070
} else {
7171
// functional
7272
const render = Component as FunctionalComponent
73+
// in dev, mark attrs accessed if optional props (attrs === props)
74+
if (__DEV__ && attrs === props) {
75+
markAttrsAccessed()
76+
}
7377
result = normalizeVNode(
7478
render.length > 1
75-
? render(props, {
76-
attrs,
77-
slots,
78-
emit
79-
})
79+
? render(
80+
props,
81+
__DEV__
82+
? {
83+
get attrs() {
84+
markAttrsAccessed()
85+
return attrs
86+
},
87+
slots,
88+
emit
89+
}
90+
: { attrs, slots, emit }
91+
)
8092
: render(props, null as any /* we know it doesn't need it */)
8193
)
8294
fallthroughAttrs = Component.props ? attrs : getFallthroughAttrs(attrs)
@@ -107,17 +119,36 @@ export function renderComponentRoot(
107119
root.patchFlag |= PatchFlags.FULL_PROPS
108120
}
109121
} else if (__DEV__ && !accessedAttrs && root.type !== Comment) {
110-
const hasListeners = Object.keys(attrs).some(isOn)
111-
warn(
112-
`Extraneous non-props attributes (` +
113-
`${Object.keys(attrs).join(', ')}) ` +
114-
`were passed to component but could not be automatically inherited ` +
115-
`because component renders fragment or text root nodes.` +
116-
(hasListeners
117-
? ` If the v-on listener is intended to be a component custom ` +
118-
`event listener only, declare it using the "emits" option.`
119-
: ``)
120-
)
122+
const allAttrs = Object.keys(attrs)
123+
const eventAttrs: string[] = []
124+
const extraAttrs: string[] = []
125+
for (let i = 0, l = allAttrs.length; i < l; i++) {
126+
const key = allAttrs[i]
127+
if (isOn(key)) {
128+
// remove `on`, lowercase first letter to reflect event casing accurately
129+
eventAttrs.push(key[2].toLowerCase() + key.slice(3))
130+
} else {
131+
extraAttrs.push(key)
132+
}
133+
}
134+
if (extraAttrs.length) {
135+
warn(
136+
`Extraneous non-props attributes (` +
137+
`${extraAttrs.join(', ')}) ` +
138+
`were passed to component but could not be automatically inherited ` +
139+
`because component renders fragment or text root nodes.`
140+
)
141+
}
142+
if (eventAttrs.length) {
143+
warn(
144+
`Extraneous non-emits event listeners (` +
145+
`${eventAttrs.join(', ')}) ` +
146+
`were passed to component but could not be automatically inherited ` +
147+
`because component renders fragment or text root nodes. ` +
148+
`If the listener is intended to be a component custom event listener only, ` +
149+
`declare it using the "emits" option.`
150+
)
151+
}
121152
}
122153
}
123154

0 commit comments

Comments
 (0)