Eslint-plugin-react: react/destructuring-assignment oneliner

[AdrienLemaire]

Simple and clean oneliner:

  handleClick = () => this.props.firebase.logout();

Right now, I have to write

  handleClick = () => {
    const {firebase} = this.props;
    firebase.logout();
  };

This destructuration is useless in this case. Please remove warnings for oneliner methods where no params are given to the method.

[ljharb]

It's definitely not useless; in this case, it doesn't cover the most dangerous problem:

handleClick = () => this.props.foo();
// passes `this.props` as the `this` value, which exposes it to code that could mutate it.

Separately, you never want to put arrow functions in class fields, so I wouldn't recommend that being a one-liner anyways :-)

[AdrienLemaire]

I should have mentioned that I had this as well in the code

  constructor(props) {
    super(props);
    this.handleClick = this.handleClick.bind(this);
  }

I thought this.props was immutable. Can you give me an example where it can get mutated @ljharb ?
Thank you for the insight!

[AdrienLemaire]

Separately, you never want to put arrow functions in class fields

That's the first time I'm hearing about this. Is there an eslint rule to cover and explain this as well ? Couldn't find it :)

[ljharb]

The props object should not be mutated, but is in no way immutable.

As for arrow functions in class fields, you always want instance methods with a constructor-binding (doing a constructor-bind along with an arrow function is redundant) for performance, clarity, and testability.

There isn't an eslint rule yet, but as soon as it's stage 4, I plan to add one to eslint core.

[AdrienLemaire]

  handleClick () {
    const {firebase} = this.props;
    firebase.logout();
  }

it is, then. Looking forward to the rule, please share it in this issue if you don't forget about it by then, thanks :)

[jeffvandyke]

@ljharb ...until decorators are also stage 4, then an @autobind on the handler is much less jumping around than binding in constructors.

[ljharb]

@jeffvandyke sure. With class fields, you can also do:

foo = this.foo.bind(this);
foo() { }