Skip to content

⭐️New: Add vue/v-on-parens rule #481

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 9 commits into from
Feb 3, 2019

Conversation

niklashigi
Copy link
Contributor

@niklashigi niklashigi commented May 27, 2018

A rule that enforces the consistent use of parentheses after method calls — not actually method calls without parentheses, but you get what I mean — in v-on directives.

  • 'always': <button @click="foo()">Foo</button>
  • 'never': <button @click="foo">Foo</button>

I think this rule is useful because it enforces consistency (always good) and when set to 'never' results in smaller output (without unnecessary "function wrapping").


The rule includes — based on my tests with real-world projects — reliable fixers.
It does not support the new 2.4.0+ v-on object syntax because other rules also don't.

I'd be happy about feedback on:

  • what the name of this rule should be
    • I don't like the current one a lot, but am not quite sure which name would be better: maybe v-on-avoid-inline-statements (that sounds better, but 'always' and 'never' no longer make that much sense)?
  • how the error messages and docs are worded
    • Are they too complicated?
  • whether this rule should be 'recommended'

@chrisvfritz
Copy link
Contributor

Hm, my initial thought is there might be two possible rules here:

  • v-on-avoid-inline-statement (values: "error", "off")
  • v-on-function-call (values: "always", "never", "only-args", "off")

I'll walk through these in a little more detail.

v-on-avoid-inline-statement

I think "avoid" is an important word here, because there will always be unavoidable exceptions when a new variable is defined in the template scope (e.g. with v-for and slot-scope). For example:

<button 
  v-for="todo in todos"
  @click="openTodoModal(todo)"
/>

The rule could even help people avoid:

<input @input="$emit('input', $event.target.value)">

in favor of:

<template>
  <input @input="handleInput">
</template>

<script>
export default {
  methods: {
    handleInput(event) {
      this.$emit('input', event.target.value)
    }
  }
}
</script>

Then I would personally argue that this is unhelpfully extreme, but I've known people (particularly coming from React) who might prefer this pattern. For people like me though, an option to whitelist certain methods, like $emit, could be helpful.

I think this rule should probably stay uncategorized for now though.

v-on-function-call

This one is similar to the first option, but covers a greater variety of use cases:

  • Some people might always want the parentheses (e.g. @click="doSomething()" instead of @click="doSomething" to make it clearer that a function is being called on click.
  • Some might only want parentheses used when the function has arguments (e.g. @click="doSomething('hello')" is OK, but not @click="doSomething()".
  • Some might never want to call a functional inline, like those I mentioned in the first rule idea.

Again, I don't think this rule should be categorized for now.

What do you think @michalsnik?

@michalsnik
Copy link
Member

I like the idea behind v-on-function-call @chrisvfritz 👍

@michalsnik
Copy link
Member

Now that I wanted to update this PR and merge, I think that current implementation is perfectly fine and I'm happy to proceed with it as is. So that we only report methods that don't have any arguments. So if anyone would want to pass argument - he/she can and the rule will not throw any warnings about parantheses being used. For that case we can have separate rule if we'll find it valuable.

@michalsnik
Copy link
Member

@armano2 @chrisvfritz can you take a look once again?

@michalsnik michalsnik self-assigned this Jan 29, 2019
@michalsnik
Copy link
Member

@ota-meshi @armano2 I updated this PR, can give it another look? :)

Copy link
Member

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@michalsnik michalsnik merged commit ae03c28 into vuejs:master Feb 3, 2019
@niklashigi niklashigi deleted the args-parens-rule branch February 3, 2019 05:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants