Update React rules

[calebcassel]

This PR includes some big updates to our React rules. 🎉

Many of these changes were done by manually merging code from Airbnb. Then I went through each Airbnb rule individually, disabled a few and adjusted the levels and configurations of others. The diff for this is difficult to look through, so I've broken down each rule change below. Please review each one and comment with any thoughts or concerns.

After bringing in the Airbnb changes, I also made some updates to the React README.md to bring it more in line with our rules. Finally, I made a few tweaks to the Docker build based on comments from my previous PR (#42). This would be the first release to use the new build.

Major Updates 💪

• eslint-config-hudl now requires ESLint 4 or 5. This was previously on 3.

New Error Rules 🛑

Violating any of these will be counted as an error.

• react/jsx-no-duplicate-props
  Don't pass the same prop to a component more than once.
• react/no-direct-mutation-state
  Never assign to this.state directly.
• react/no-will-update-set-state
  No calls to this.setState() within componentWillUpdate()
• react/no-is-mounted
  Don't call this.isMounted(). Nobody should be doing this at this point anyway.
• react/prefer-es6-class
  Don't use React.createClass(). This was deprecated in React 15.5.
• react/require-render-return
  Render method must always have a return statement
• react/no-render-return-value
  Don't use the return value of ReactDOM.render(). Use a ref instead.
• react/no-comment-textnodes
  Prevent accidental comments in JSX that get rendered as actual text.
• react/no-danger-with-children
  Don't use dangerouslySetInnerHTML when the component has child content
• react/no-typos
  Prevent typos when casing common React properties or methods, such as propTypes or componentWillUpdate.
• react/no-this-in-sfc
  Prevent this from being used in a stateless functional component.
• react/no-access-state-in-setstate
  This is an anti-pattern that can be easily avoided.
• react/void-dom-elements-no-children
  Certain elements, such as <br/>, <img/>, <hr/>, should not have children.
• react/default-props-match-prop-types
  Default props should always be specified in propTypes too.
• react/no-redundant-should-component-update
  Components extending PureComponent should not implement a shouldComponentUpdate.
• react/jsx-props-no-multi-spaces
  Only have one space between JSX props.
• react/no-children-prop
  Children should be passed as child elements, not as a children prop.
• react/no-unused-state
  Prevent setting state values if that value is never read.
• react-hooks
  Enforce React hooks rules.

Fixable Errors 🔧

Each of these rules will also produce an error, but can be fixed automatically by running ESLint with the --fix parameter. These are all related to JSX formatting.

• jsx-quotes
  Require double quotes in JSX attributes.

• react/jsx-equals-spacing
  There should be no spaces around JSX equals signs, such as:
  <Hello name = {firstname} />
<Hello name ={firstname} />
<Hello name= {firstname} />

• react/jsx-indent
  JSX should be indented with 2 spaces.

• react/jsx-closing-tag-location
  Closing tags should be aligned with or on the same line as the opening tag.

• react/jsx-tag-spacing
  Prevent spaces in JSX brackets, such as:

  • < Hello></Hello>
  • <Hello>< /Hello>
  • <Hello></Hello >

  A trailing space in a self-closing tag is okay: <Hello />

Updated rules ♻️

• react/sort-comp
  We were previously enforcing an order of:
  • static-methods
  • lifecycle
  • everything-else
  • render

This PR adds two new cases to this rule:

• instance variables must go after static-methods, but still before the constructor or lifecycle methods
• any render* method must go at the bottom, before the final render()

New Warnings ⚠️

These are the more egregious issues that may be much more involved to fix. If these were full errors, it may make it difficult to update a repository to the new ESLint rules without having to disable several lines and then forgetting about those violations later. For that reason I have them configured as warnings, but I'm open to any pushback on this.

• react/jsx-no-bind
  Warn for new functions that are passed as props. This could be a function(), an arrow, or use of .bind(). This is an anti-pattern.
• react/no-string-refs
  Using strings as refs is considered a React legacy feature.
• react/no-find-dom-node
  Don't use findDOMNode(). This is an anti-pattern.
• react/no-array-index-key
  Keys should be based on an item property rather than their index in the array. This is an anti-pattern.

[Vannevelj]

I think the react/no-unused-state will cause errors in a piece of VSPA code around drawing on clips:

https://github.com/hudl/hudl-videospa/blob/4e26822664928e8fa0baddf28fd167876280ae83/src/client-app/app/shared/components/Drawing/Canvas/index.jsx#L57

It exposes the component's state by wrapping it in a function and passing that to classes outside of it. The implementation of the canvas specific for effects does not have this and it's all as it should be there but that would be one place with existing code that nobody will want to touch. That's not to say I don't think it's a valid rule, just something to be aware of. If you're going to have that as an error, there will be some extensive refactoring needed for that area.

[calebcassel]

@Vannevelj Good to know. That sounds like a legitimate place we could use /* eslint-disable react/no-unused-state */ to let the build pass, as long as we all agree that this should apply generally and there aren't too many situations like that one.