-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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
improve warning when extra event listeners (fix #1001) #1005
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I accidentally worked on the same thing but I think this is more comprehensive. Can you resolve the conflict please? Thanks!
`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.` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add if the listener is intended to be a component custom event listener only, declare it using the "emits" option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added.
result = normalizeVNode( | ||
render.length > 1 | ||
? render(props, { | ||
attrs, | ||
get attrs() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should only make attrs
a getter in dev.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed. Is it okay, though, to use __DEV__
with a ternary to achieve that?
I'm not very experienced so, not sure if it helps, or if this is the correct place, but just in case... I came across this issue when googling the following warning:
After looking here and at some articles about Vue 3, I came to the conclusion that adding
would be a good idea. That got rid of the warnings. A small point of confusion for me was why it still worked even without the emits property. Also, why this only happened when I added a grand-child component where the middle component just basically passed along the emitted event (no payload). I don't expect to receive explanations here. Just hoping to do my part and provide useful (hopefully) feedback. Please let me know if I should refrain or direct them elsewhere. Thanks again for the hard work on a great framework! |
Hello, I think this error message could be improved to also say something like "you may not have intended to have this listener fallthrough; consider |
This is meant to address #1001 by separating out onXXX attribute keys when fall-through can't be performed due to multiple root nodes. It warns that there are extra non-emits event listeners, indicating to the user that they should consider adding the events to the emits option.
This PR also adds logic to call
markAttrsAccessed
when rendering functional components that have either optional props (so technically there are no extra attrs) or that accesscontext.attrs
similar to how stateful components work. This prevents the warning in those cases.