Skip to content

improve warning when extra event listeners (fix #1001) #1005

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Apr 20, 2020
72 changes: 69 additions & 3 deletions packages/runtime-core/__tests__/rendererAttrsFallthrough.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: () => {} })
}
}

Expand All @@ -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: () => {} })
}
}

Expand All @@ -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: () => {} })
}
}

Expand All @@ -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')
Expand Down
4 changes: 3 additions & 1 deletion packages/runtime-core/src/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,9 @@ function finishComponentSetup(

const attrHandlers: ProxyHandler<Data> = {
get: (target, key: string) => {
markAttrsAccessed()
if (__DEV__) {
markAttrsAccessed()
}
return target[key]
},
set: () => {
Expand Down
63 changes: 47 additions & 16 deletions packages/runtime-core/src/componentRenderUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.`
)
}
}
}

Expand Down