Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Open
ByScripts wants to merge 2 commits into vuejs:master
base: master
Choose a base branch
Loading
from ByScripts:feature/#2460-allow-inline-inline-function-v-on-handler-style

Conversation

Copy link

@ByScripts ByScripts commented Jun 6, 2024

Fixes #2460

Allow using ["inline", "inline-function"] option for v-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++"

andreww2012 and Ericlm reacted with thumbs up emoji
Copy link
Member

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?

Copy link
Author

ByScripts commented Jun 6, 2024
edited
Loading

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?

Copy link
Member

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.

Copy link
Author

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 is true:

    () => handleFilter()
    => handleFilter() if ['inline', 'inline-function']
    => handleFilter if ['method', 'inline-function']

    (filter) => handleFilter(filter)

    (filter, negate) => handleFilter(filter, negate)

  • When allowInlineFuncSingleArg is false (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)

andreww2012 reacted with thumbs up emoji

@ByScripts ByScripts force-pushed the feature/#2460-allow-inline-inline-function-v-on-handler-style branch from e95c657 to 91da522 Compare June 8, 2024 18:41
Copy link
Member

FloEdelmann commented Jun 19, 2024
edited
Loading

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:

  1. For cases like @click="(arg1, arg2) => handler(arg1, arg2)", always allow the inline-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
  2. Add a new option ["inline", "inline-function"] that allows inline but disallows using $event, and allows using inline-function in that case.

What do you think?

Copy link
Author

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:

  1. @click="count++" ➡️ Disallow with vue/event-no-inline-expr
  2. @click="increase" ➡️ Disallow with vue/event-no-method-handler
  3. @click="increase()" ➡️ Disallow with vue/event-no-inline-handler
  4. @click="() => increase()" ➡️ Disallow with vue/event-no-arg-less-inline-function
  5. @click="increase($event)" ➡️ Disallow with vue/no-dollar-event
  6. @click="(count) => increase(count)" ➡️ No reason to disallow?
  7. @click="(count, multiply) => increase(count, multiply)" ➡️ No reason to disallow?

This is just a quick overview, and I may be overlooking certain situations.

Copy link
Member

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.

Copy link
Member

@ByScripts are you interested in changing the rule like I described in my comment above?

Copy link

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

Cached:
Screenshot 2025年02月27日 at 7 54 12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Suggestion: allow ["inline", "inline-function"] option for v-on-handler-style

AltStyle によって変換されたページ (->オリジナル) /