Anti-patterns

Prerequisites

Destructuring assignment - MDN web docs https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment

Avoid inline lambdas or binding in render()

A common mistake to make when coding in React, is to have an inline lambda in you render() function. There are multiple reason that could push you towards writing something like the following:

class MyComponent extends React.Component {
  render() {
    // bad
    return (
      <div>
        <button onClick={() => this.handleClick()} />
        <button onClick={this.handleClick.bind(this)} />
      </div>
    );
  }
}

You usually find yourself doing this when you want to have access to your component instance when calling your function. The problem with this is mostly its inefficiency (see more on Performance Optimizations)

One preferred way to fix this problem is to bind your function in your component constructor:

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

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

  handleClick() {
    console.log('I was clicked');
  }

  render() {
    return <button onClick={this.handleClick} />;
  }
}

The way we solve this problem in Shopify's codebase when using TypeScript, is by using decorators and our autobind javascript utility.

import {autobind} from '@shopify/javascript-utilities/decorators';
class MyComponent extends React.Component {
  @autobind
  handleClick() {
  }
}

You can learn more in the Performance Optimizations chapter.

Other resources: StackOverflow answer https://stackoverflow.com/questions/36677733/why-shouldnt-jsx-props-use-arrow-functions-or-bind/36677798#36677798

React Binding Patterns: 5 Approaches for Handling this https://medium.freecodecamp.org/react-binding-patterns-5-approaches-for-handling-this-92c651b5af56

Never mutate the state

Don't do mutations when setting new state.

Let's say you have an array in your state where on user event, you are going to add an item to this array. Instinctively, you might want to just push the new item in the array like that for instance:

class MyComponent extends React.Component {
  defaultState = {
    list: [],
  };

  handleChange(event) {
    const {list} = this.state;
    list.push(event.target.value);

    this.setState({
      list: list,
    });
  }
}

The problem here is that you are mutating the current state and passing to the new state a reference to the old list. The problem here, is that when React will compare the new state with the last state to render your component, it will get confused since it is the same reference.

What you want to do instead is passing a new reference to React. To do so, you can do following:

handleChange(event) {
  const {list} = this.state;
  // Creates a new Array
  const newList = [...list, event.target.value]; 

  this.setState({
    list: newList,
  });
}

You can learn more on this new ES6 synthax on: Destructuring assignment - MDN web docs.

🙅🏻 Update Component props

Updating directly the component props like the following code, is something you technically can't do. Props are read-only, and the compiler will fail if you try to re-defined them with something along the lines of TypeError: Cannot assign to read only property

  handleChange(event) {
    this.props.userValue = event.target.value;
  }

The React philosophy is that props should be immutable and top-down. This means that a parent can send whatever prop values it likes to a child, but the child cannot modify its own props.

You only ever update your own state, and react to prop values you are given by parent.

If you want to have an action occur on a child which modifies something on the state, then what you do is pass a callback to the child which it can execute upon the given action. This callback can then modify the parent's state, which in turns can then send different props to the child on re-render.

Detailed answer on StackOverflow https://stackoverflow.com/a/26089687/900301

Passing className to children as props

TL:DR; Use pre-defined variations instead of passing down classNames to a component

Passing classes down as props defeats the purpose of encapsulation. Passing CSS classes down to children component is something to avoid since it defeats the purpose of encapsulation. If you find yourself in this situation, try to think of how to customize your component using pre-defined variations instead.

If we take the example of an Alert banner, instead of doing:

  <Alert classes={styles.RedBackground}>

You could do something like:

  <Alert type={'error'}>