Movatterモバイル変換


[0]ホーム

URL:


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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
ByScripts wants to merge2 commits intovuejs:master
base:master
Choose a base branch
Loading
fromByScripts:feature/#2460-allow-inline-inline-function-v-on-handler-style

Conversation

@ByScripts
Copy link

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++"

andreww2012 and Ericlm reacted with thumbs up emoji
@FloEdelmann
Copy link
Member

Cool, thank you! Should theinline-function style really already be accepted with one argument (whereinline would also work with$event), or only with two or more?

@ByScripts
Copy link
Author

ByScripts commentedJun 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?

@FloEdelmann
Copy link
Member

I think if you want to force explicit parameter naming, then you should only allowinline-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 toinline-function wheninline does not work at all, i.e. only for functions with more than one parameter.

@ByScripts
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 anallowInlineFuncSingleArg?: boolean option that can only be used in conjunction with['method', 'inline-function'] or['inline', 'inline-function'].

  • WhenallowInlineFuncSingleArg istrue:

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

    (filter) => handleFilter(filter)

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

  • WhenallowInlineFuncSingleArg 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)

andreww2012 reacted with thumbs up emoji

@ByScriptsByScriptsforce-pushed thefeature/#2460-allow-inline-inline-function-v-on-handler-style branch frome95c657 to91da522CompareJune 8, 2024 18:41
@FloEdelmann
Copy link
Member

FloEdelmann commentedJun 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 theinline-function style, because there is no other way to represent those. Multiple event payloads should rather be reported at the emitting side, seeRule suggestion:vue/prefer-single-payload-in-event #2005
  2. Add a new option["inline", "inline-function"] that allowsinline but disallows using$event, and allows usinginline-function in that case.

What do you think?

@ByScripts
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 thinkvue/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 withvue/event-no-inline-expr
  2. @click="increase" ➡️ Disallow withvue/event-no-method-handler
  3. @click="increase()" ➡️ Disallow withvue/event-no-inline-handler
  4. @click="() => increase()" ➡️ Disallow withvue/event-no-arg-less-inline-function
  5. @click="increase($event)" ➡️ Disallow withvue/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.

@ota-meshi
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 thev-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.

@FloEdelmann
Copy link
Member

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

@jiikoosol
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 freeto 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 forv-on-handler-style

4 participants

@ByScripts@FloEdelmann@ota-meshi@jiikoosol

[8]ページ先頭

©2009-2025 Movatter.jp