-
-
Notifications
You must be signed in to change notification settings - Fork 694
feat(v-on-handler-style): allow ["inline", "inline-function"]
option
#2471
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
feat(v-on-handler-style): allow ["inline", "inline-function"]
option
#2471
Conversation
Cool, thank you! Should the inline-function
style really already be accepted with one argument (where inline
would also work with $event
), or only with two or more?
This would prevent the interdiction to use $event
in favor of explicit naming:
<template> <!-- Using an inline function with a good argument name is better --> <MyComp @increase="(count) => myHandler(count)" />` <!-- It's why we don't allow $event, as it is meaningless (and is absolutely not an Event here) --> <MyComp @increase="myHandler($event)" />` </template>
Edit: Or maybe it could be configurable as an option?
I think if you want to force explicit parameter naming, then you should only allow inline-function
. It's 5 extra characters for functions without parameters (() =>
), but it's a simple and effective rule.
If you prefer shortness over explicitness however, then ["inline", "inline-function"]
is fine, but IMO it should only fall back to inline-function
when inline
does not work at all, i.e. only for functions with more than one parameter.
I see your point, but it's really common to only need an inline handler.
I don't think we should force users to use @event="() => handler()"
where @event="handler()"
is enough.
The primary goal of this PR is to forbid the usage of method handler, as it's error prone, and allow the two others.
I updated my proposal to add an allowInlineFuncSingleArg?: boolean
option that can only be used in conjunction with ['method', 'inline-function']
or ['inline', 'inline-function']
.
-
When
allowInlineFuncSingleArg
istrue
:❌
() => handleFilter()
=>handleFilter()
if['inline', 'inline-function']
=>handleFilter
if['method', 'inline-function']
✅
(filter) => handleFilter(filter)
✅
(filter, negate) => handleFilter(filter, negate)
-
When
allowInlineFuncSingleArg
isfalse
(by default)❌
() => handleFilter()
=>handleFilter()
if['inline', 'inline-function']
=>handleFilter
if['method', 'inline-function']
❌
(filter) => handleFilter(filter)
=>handleFilter($event)
=>handleFilter($event)
if['inline', 'inline-function']
=>handleFilter
if['method', 'inline-function']
✅
(filter, negate) => handleFilter(filter, negate)
e95c657
to
91da522
Compare
Hmm, I don't like the extra option. The logic is quite convoluted already anyway, and the extra option makes it even harder to understand.
But I think we can change the rule like this:
- For cases like
@click="(arg1, arg2) => handler(arg1, arg2)"
, always allow theinline-function
style, because there is no other way to represent those. Multiple event payloads should rather be reported at the emitting side, see Rule suggestion:vue/prefer-single-payload-in-event
#2005 - Add a new option
["inline", "inline-function"]
that allowsinline
but disallows using$event
, and allows usinginline-function
in that case.
What do you think?
Thank you for your reply.
After some thought, I wonder if this rule isn't trying to handle too many things at once.
I agree on your first point (although I think vue/event-prefer-single-payload
would be a better name. At first I read "Prefer Single-Event Payload" ^^)
Perhaps it would be easier to disallow these different behaviors with new, independent rules?
For example:
@click="count++"
➡️ Disallow withvue/event-no-inline-expr
@click="increase"
➡️ Disallow withvue/event-no-method-handler
@click="increase()"
➡️ Disallow withvue/event-no-inline-handler
@click="() => increase()"
➡️ Disallow withvue/event-no-arg-less-inline-function
@click="increase($event)"
➡️ Disallow withvue/no-dollar-event
@click="(count) => increase(count)"
➡️ No reason to disallow?@click="(count, multiply) => increase(count, multiply)"
➡️ No reason to disallow?
This is just a quick overview, and I may be overlooking certain situations.
Thanks for working on new option and for the suggestions.
However, for now I don't think it's a good idea to split up what the v-on-handler-style
rule is doing, because I think it would be hard to design the options in such a way that those rules don't conflict.
@ByScripts are you interested in changing the rule like I described in my comment above?
jiikoosol
commented
Feb 27, 2025
I just stumbled on this one when going through a larger codebase which has been using the method-style when defining v-on event handlers. Shouldn't the vue/v-on-handler-style rule by default forbid using the method style as in Vue 3 the onClick event is not cached when the function is typed without parentheses?
Here's an example:
Not cached:
Screenshot 2025年02月27日 at 7 54 21
Fixes #2460
Allow using
["inline", "inline-function"]
option forv-on-handler-style
rule.In this case:
method
style will be forbidden@click="myHandler"
inline-function
style will only be accepted with at least 1 argument:@click="() => myHandler()"
@click="myHandler()"
@click="(evt) => myHandler(evt)"
@click="(arg1, arg2) => myHandler(arg1, arg2)"
inline
style will be required in other cases@click="() => myHandler($event)"
@click="myHandler($event)"
@click="() => count++"
@click="count++"