Billy Rhoades' Blog

React Pitfalls

Introduction #

React has some common pitfalls that I've seen developers fall into when designing components. The most common center around readability and performance. Now, there's easily a dozen articles about React best practices but these lean abstract with contrived examples. Personally, I find it much easier to see where people made mistakes, see how we fixed them, and then break those examples down.

Nested Component Definitions #

Defining components within components breaks reusability, causes unnecessary rendering, and crowds files. It's particularly tempting to nest components when one component needs variables from the other, but it leads to coupling. Nesting components will always lead to rerenders of child compoents when any prop in the parent component is changed. Fortunately, it's easy to fix and fairly easy to diagnose.

Bad #

const Section = ({ description, footer, header, items, sectionId, onChange }) => {
  const Item = ({ content, label, index, value }) => (
    <React.Fragment>
      <Header size="small">{label}</Header>
      {content}
      <Input onChange={(e) => onChange(sectionId, index, e.target.value)} value={value} />
      <Spacer size="small" />
    </React.Fragment>
  );
  
  const Content = () => (
    items.map(({ content, label }, i) => (
      <Item
        content={content}
        label={label}
        index={i}
        key={`${sectionId}${label}`}
      />
    ));
  );
  
  return (
    <Card>
      <Header>{header}</Header>
      <Content />
      <Footer>{footer}</Footer>
    </Card>
  );
});

Here you can see a section component which, within it, defines an Item component and a Content component. Defining an internal Content component here is only a coupling issue. However, defining Item within this component is going to cause some pain. Item is defined within this component because it's only used by Section and it needs sectionId and onChange from Section.

However, defining the component within Section causes Item to be redefined and rerendered every time any Section prop changes. Any Inputs on the page will be destroyed and mounted again within the DOM. If you were typing into an Input component and a Section prop changed, your focus would be lost. I've ran into this scenario quite a few times--- onChange updates Section's items which causes a rerender.

If Item were moved outside of Section, React would behave differently.

Good #

const Item = ({ content, label, index, onChange, sectionId }) => (
  <React.Fragment>
    <Header size="small">{label}</Header>
    {content}
    <Input onChange={(e) => onChange(sectionId, index, e.target.value)} />
    <Spacer size="small" />
  </React.Fragment>
);

const Section = ({ description, footer, header, items, sectionId, onChange }) => {
  const content = items.map(({ content, label }, i) => (
    <Item
      content={content}
      label={label}
      index={i}
      sectionId={sectionId}
      onChange={onChange}
      key={`${sectionId}${label}`}
    />
  ));

  return (
    <Card>
      <Header>{header}</Header>
      {content}
      <Footer>{footer}</Footer>
    </Card>
  );
});

As mentioned above, with Item defined outside of section any changes to Section's props which don't affect Item's props will not cause a rerender. Only components with modified props will be updated. Input's focus will not be lost as long as its props aren't changed. We save unnecessary renders here and remove future coupling issues.

Abusing Keys #

Keys in React are used when returning procedurally generated components. One of the most common uses is an array of values which will be individually passed into equally many components, as can be seen below. React's documentation describes this well:

Keys help React identify which items have changed, are added, or are removed. Keys should be given to the elements inside the array to give the elements a stable identity.

Unfortunately, keys aren't always available inside a data source. When not available, keys can be chosen that aren't stable between renders or not unique among sibling components. React's documentation mentions this:

Keys should be stable, predictable, and unique. Unstable keys (like those produced by Math.random()) will cause many component instances and DOM nodes to be unnecessarily recreated, which can cause performance degradation and lost state in child components.

In a pinch, using the array index may suffice. However, I'm in the camp that index as a key is an antipattern; you only do this until you encounter a bug from it. Bugs resulting from index as a key are incredibly difficulty to diagnose; they usually arise from where components that have state are change keys. The bug's symptoms will be varied: maybe state tranfers when an element is deleted from an index, or maybe it gets a random state from an older component. You don't want to learn the symptoms.

Bad #

const shortid = require('shortid');

const Quiz = ({ sections }) => {
  return (
    <div>
      {
        sections.map((section) => (
          <Section
            {...section}
            key={shortid.generate()}
          />
        ))
      }
    </div>
  );
});

We're using shortid to generate a unique, per render random key for our sections. This would be a nonissue if sections never change position or the list never changes size. However, if sections does change, it's possible that there could be performance issues or state bugs. This can sometimes be easy to solve... sometimes it's not.

Good #

const Quiz = ({ sections }) => {
  return (
    <div>
      {
        sections.map((section) => (
          <Section
            {...section}
            key={section.label}
          />
        ))
      }
    </div>
  );
});

The shortid is now swapped for label, which in this case is both unique among all sections and consistent. If you can't find a key that's unique, you may find yourself resorting to using an index, but that should be a last resort.

Redux: Overloading mapStateToProps #

With Redux, mapStateToProps is used to subscribe to updates from the store. Any time the store is updated, every component decorated with connect will have its mapStateToProps called. The output of mapStateToProps is then passed to the component via props.

Since mapStateToProps is called on every update, React applications which heavily utilize the store could see a performance impact. For example, if you are storing the contents of a form in Redux, every letter typed into a form by a user may trigger a store update. Depending on how your application is laid out, this would trigger every mapStateToProps on every keypress, and possibly trigger rerenders if those result in prop changes.

If there are any mapStateToProps functions that do expensive calculation, such as computing the initial values for the form which a user is inputting into, those will be recalculated every time a change occurs. Since the initial state of a form likely wouldn't change from user input, it wouldn't affect the output for mapStateToProps here. However, the calculation would still happen under the hood.

One workaround here is to use Reselect in any place where you are performing complex logic within mapStateToProps. Reselect, which behaves very similarly to mapStateToProps, creates a selector. When defining a selector, it needs a list of input functions which will be used to get a list of outputs. The list of outputs are then passed to a result function, which should have any expensive computation in it. From Reselect's documentation:

// createSelector(...inputSelectors | [inputSelectors], resultFunc)

const totalSelector = createSelector(
  [
    state => state.values.value1,
    state => state.values.value2
  ],
  (value1, value2) => value1 + value2
)

This example is very simple, but it demonstrates basic usage well. Here we created a selector with two input selectors that grab values from the store. These values are passed through to the result function passed as a second argument to createSelector. totalSelector would be used in your mapStateToProps:

mapStateToProps = (state) => ({
  totalOutput: totalSelector(state),
});

totalSelector is memoized; it is only called when one of its input selectors return a different value. So if value1 + value2 were an expensive operation that rarely occurred, it would only be called if the two input selectors had changed. This stops expensive computations from occurring when they will provide the same output.

Resources #