Skip to content
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

Exempt variables prefixed with underscore from no-unused-vars rule #640

Merged
merged 2 commits into from
Sep 16, 2016

Conversation

valscion
Copy link
Contributor

@valscion valscion commented Sep 13, 2016

This is useful when e.g. using object spread operator to remove only a
certain field from the object.

For example, this can be used to ignore a property from React
component's this.props:

class OpinionatedDebugger extends React.Component {
  render() {
    const { justIgnoreMe: _unused, ...rest } = this.props;
    return <pre>{ JSON.stringify(rest) }</pre>;
  }
}

This PR was asked by Dan via Twitter when he replied to my tweet about ignoring unused variables with underscore prefix: https://twitter.com/dan_abramov/status/775764929894809602

Test plan:

Write this to any JS file in the project and observe no warnings are shown:

var obj = { a: 1, b: 2 };
var { a: _unused, ...rest } = obj;
console.log(rest);

screen shot 2016-09-13 at 22 45 01

Then try to change it to this and observe warnings are shown:

var obj = { a: 1, b: 2 };
var { a, ...rest } = obj;
console.log(rest);

screen shot 2016-09-13 at 22 45 14

## Same approach elsewhere

This approach is similar in nature to what rubocop, the de-facto linter in Ruby, specifies in its default config as well: https://github.com/bbatsov/ruby-style-guide#underscore-unused-vars

Prefix with _ unused block parameters and local variables. It's also acceptable to use just _(although it's a bit less descriptive). This convention is recognized by the Ruby interpreter and tools like RuboCop and will suppress their unused variable warnings.

@ghost ghost added the CLA Signed label Sep 13, 2016
This is useful when e.g. using object spread operator to remove only a
certain field from the object.

For example, this can be used to ignore a property from React
component's `this.props`:

    render() {
        const { someAttribute: _unused, ...rest } = this.props;
        return <pre>{ JSON.stringify(rest) }</pre>;
    }
@valscion
Copy link
Contributor Author

Let me know if you'd like this change to also be documented somewhere :)

@ghost ghost added the CLA Signed label Sep 13, 2016
@gaearon
Copy link
Contributor

gaearon commented Sep 13, 2016

Let's mention this somewhere in the usage guide.

@valscion
Copy link
Contributor Author

I'm looking at the template/README.md file and I feel a bit unsure where this would fit in. This change is quite small so I had hoped there would've been an existing section for linting or object spreads but there isn't one.

That file itself is already quite large so that makes me even more wary about tucking in this little detail as I fear I'd too much irrelevant details.

What would a section covering this lint rule have? Should it contain more details about the linting in general, or just a one sentence note about this feature?

@vjeux
Copy link
Contributor

vjeux commented Sep 15, 2016

(FWIW, we have the same lint rule on our PHP codebase at FB)

@valscion
Copy link
Contributor Author

I'm quite stuck here, I don't know how to document this. Any help would be greatly appreciated 💕

@gaearon
Copy link
Contributor

gaearon commented Sep 16, 2016

Ah, okay, let’s just sneak it in and maybe document it in some way when we release a package of our lint rules separately.

@gaearon gaearon added this to the 0.4.2 milestone Sep 16, 2016
@gaearon gaearon merged commit e333b8b into facebook:master Sep 16, 2016
@gaearon
Copy link
Contributor

gaearon commented Sep 16, 2016

Thank you!

@gaearon gaearon mentioned this pull request Sep 18, 2016
@valscion
Copy link
Contributor Author

Ah, okay, let’s just sneak it in and maybe document it in some way when we release a package of our lint rules separately.

That sounds cool! 👍

@valscion valscion deleted the no-warn-underscore-prefix branch September 18, 2016 13:14
feiqitian pushed a commit to feiqitian/create-react-app that referenced this pull request Oct 25, 2016
…acebook#640)

* Split no-unused-vars ESLint config to multiple lines

* Exempt variables prefixed with underscore from no-unused-vars rule

This is useful when e.g. using object spread operator to remove only a
certain field from the object.

For example, this can be used to ignore a property from React
component's `this.props`:

    render() {
        const { someAttribute: _unused, ...rest } = this.props;
        return <pre>{ JSON.stringify(rest) }</pre>;
    }
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants