Minor cleanup for the React styleguide

[kesne]

This fixes some minor things that I thought were missing from the styleguide.

If people are unhappy about enforcing names for event handlers or additional render methods, I'm happy to discuss/remove.

---

• Obey our own styleguide for modules.
• Added missing rule for true props.
• New rule on event handler binding.

[ljharb]

can we omit the extension in all our examples? file extensions should ideally never be specified in a require

[koistya]

@ljharb 👍

[koistya]

Prefix the name of event handlers with on.

<button onClick={handleClick}>Click me</button>
           │          │
           │          └─ (verb) component's action
           └─ (noun) component's property

In many cases the event handlers (actions) should be named so that it's easy to understand what a particular function is doing. For example:

<button onClick={pokePlayer}>Poke Player</button>
           │          │
           │          └─ (verb) a function performing a specific action
           └─ (noun) a property holding a reference to the component's action(s)

In both these examples using the on prefix for action names (as this PR suggests) doesn't make any sense to me. I would rather prefix my components' actions with verbs (handleClick, deleteUser, closeModal etc.).

[ljharb]

That is a pretty convincing counterpoint.

[kesne]

The one issue with that is it makes the sort-comp rule pretty useless, assuming those methods exist on the component.

[ljharb]

That is true. However, I'm not convinced we want to retain that rule (obv it'll need further discussion)

[kesne]

I'm happy to drop that from this PR and leave it for another conversation.

[ljharb]

@kesne yeah let's split the naming section into a separate PR, and keep the rule fix and the rest of the guide cleanups?

[charpeni]

I think we should use arrow functions instead of .bind(this), it's more ES6.

// bad
  class extends React.Component {
    constructor(props) {
      super(props);

      this.onClickDiv = this.onClickDiv.bind(this);
    }

    onClickDiv() {
      // do stuff
    }

    render() {
      return <div onClick={this.onClickDiv} />
    }
  }

// good
  class extends React.Component {
    constructor(props) {
      super(props);
    }

    onClickDiv = () => {
      // do stuff
    }

    render() {
      return <div onClick={this.onClickDiv} />
    }
  }

[ljharb]

@charpeni That would require the class properties proposal, which we don't yet use at Airbnb because it's too far from being finalized.

[goatslacker]

it's more ES6.

It's actually not even ES7 😭

[charpeni]

@ljharb I did not noticed that class properties was only a proposal, thank you for the clarification.

[ljharb]

LGTM pending one final rebase. (This is a breaking change, fwiw)