Case Study: The Dangers of Mutating State in React

"Don't directly mutate state" is a common and fundamental recommendation in the React community. But for many beginning developers, it's not always obvious why they should avoid it or even when they're doing it. And sometimes, the bug it causes may be so sneaky that it stays hidden until long after you've deployed the code. In this post, I'll show you an example of a mutation-related bug we found in a real, live production app and explain how we fixed it.

Background

Like many apps, this one has a lot of forms. To make it easier for developers to add and update these forms, they created a set of reusable low-level components designed to interact with a form management library like formik. One feature that frequently appeared in these forms was a dynamic array of sub-forms. Imagine a list of contacts, where each contact has a name, email address, and phone number. You can update a specific field for an existing contact, delete a contact, or add a new contact.

Because this pattern was so prevalent, they created a dedicated ArrayField component to handle the common tasks of adding, removing, and updating items. To make it reusable, you define the component it uses to render the sub-form. Take a look at the demo and a simplified version of the implementation below.

import * as React from 'react'

export type ArraySubFieldProps<T> = {
  label: string
  value: T
  index: number
  onUpdate: (index: number, value: T) => void
  onRemove: (index: number) => void
  onReset: (index: number) => void
}

export type ArrayFieldProps<T> = {
  label: string
  itemLabel?: string
  component: React.ElementType<ArraySubFieldProps<T>>
  defaultValue: T
  value: T[]
  onChange: React.Dispatch<React.SetStateAction<T[]>>
}

export function ArrayField<T>({
  label,
  itemLabel = 'Item',
  component: Component,
  defaultValue,
  value,
  onChange,
}: ArrayFieldProps<T>) {
  // These callbacks are only used within this component,
  // so no need to memoize them.
  const appendItem = () => {
    onChange([...value, defaultValue])
  }

  // These callbacks get passed to children, so we should
  // memoize them in case that affects the child component.
  const removeIndex = React.useCallback(
    (index: number) => {
      onChange((value) => {
        const nextValue = [...value]
        nextValue.splice(index, 1)
        return nextValue
      })
    },
    [onChange]
  )

  const resetIndex = React.useCallback(
    (index: number) => {
      onChange((value) => {
        const nextValue = [...value]
        nextValue[index] = defaultValue
        return nextValue
      })
    },
    [onChange, defaultValue]
  )

  const updateItem = React.useCallback(
    (index: number, nextItem: T) => {
      onChange((value) => {
        const nextValue = [...value]
        nextValue[index] = nextItem
        return nextValue
      })
    },
    [onChange]
  )

  return (
    <div>
      <h2>{label}</h2>
      <ul>
        {value.map((item, i) => (
          <li key={i}>
            <Component
              label={`${itemLabel} #${i + 1}`}
              value={item}
              index={i}
              onUpdate={updateItem}
              onRemove={removeIndex}
              onReset={resetIndex}
            />
          </li>
        ))}
      </ul>
      <button onClick={appendItem}>Add {itemLabel}</button>
    </div>
  )
}

Users

Links

Now, whenever you have a new type of sub-form to render, you simply create a component for that form and handle the props it receives.

import * as React from 'react'
import type { User } from './UserForm'
import { UserForm } from './UserForm'
import type { Link } from './LinkForm'
import { LinkForm } from './LinkForm'

const defaultUser: User = {
  firstName: '',
  lastName: '',
  email: '',
}
const defaultLink: Link = {
  url: '',
  description: '',
}

export function DemoForm() {
  const [users, setUsers] = React.useState<User[]>([])
  const [links, setLinks] = React.useState<Link[]>([])

  return (
    <div>
      <ArrayField
        label="Users"
        itemLabel="User"
        component={UserForm}
        defaultValue={defaultUser}
        value={users}
        onChange={setUsers}
      />
      <ArrayField
        label="Links"
        itemLabel="Link"
        component={LinkForm}
        defaultValue={defaultLink}
        value={links}
        onChange={setLinks}
      />
    </div>
  )
}

Notice that there are a few "unspoken" rules in place with this pattern. One of these rules is that the defaultValue prop also needs to be memoized for the memoized callbacks to remain stable. In this case, we achieve that by defining the objects outside the component body, but you could also do it with the React.useMemo hook.

Recreating the Bug

You might not have run into the bug while trying out the demo form. But it's there, waiting, and it'll wait weeks, months, or even years before showing itself (probably during a sales demo when the stakes are extra high).

If you didn't run into it above, try again here. But this time, try adding multiple users and changing some fields. Also, see what happens if you try to add another user after changing a user you've already added.

Users

Pretty surprising, right?

Before we dig into what happened, take a look at the UserForm implementation and see if you can spot the bug.

import * as React from 'react'
import type { ArraySubFieldProps } from './ArrayField'

export type User = {
  firstName: string
  lastName: string
  email: string
}

export function UserForm({
  label,
  value,
  index,
  onUpdate,
  onRemove,
  onReset,
}: ArraySubFieldProps<User>) {
  const changeValue = (event: React.FormEvent) => {
    const target = event.target as HTMLInputElement
    const nextValue = value
    nextValue[target.name as keyof User] = target.value
    onUpdate(index, nextValue)
  }

  return (
    <fieldset>
      <legend>{label}</legend>
      <label htmlFor={`firstName-${index}`}>First Name</label>
      <input
        id={`firstName-${index}`}
        name="firstName"
        type="text"
        value={value.firstName}
        onChange={changeValue}
      />
      <label htmlFor={`lastName-${index}`}>Last Name</label>
      <input
        id={`lastName-${index}`}
        name="lastName"
        type="text"
        value={value.lastName}
        onChange={changeValue}
      />
      <label htmlFor={`email-${index}`}>Email</label>
      <input
        id={`email-${index}`}
        name="email"
        type="email"
        value={value.email}
        onChange={changeValue}
      />
      <button onClick={() => onRemove(index)}>Remove</button>
      <button onClick={() => onReset(index)}>Reset</button>
    </fieldset>
  )
}

Tracking Down the Bug

The trick used in the onChange callback has become fairly popular in the past few years, for good reason. If you're unfamiliar, it relies on setting the name prop for each input to match the name of its property in the value object. You can then grab the name from the event's target in the callback and dynamically update the value on the original object. It's helpful because not only can it reduce a potentially large number of similar callbacks (changeFirstName, changeLastName, changeEmail, etc.) down to one flexible callback, but it also aligns pretty well with how form elements work in vanilla JavaScript apps.

In this case, the issue is how we update a property on the original object. It's a mistake I see pretty often: either the author forgot to make a copy of the original, or (if they're not yet familiar with how object references work in JavaScript) they may have thought they already had made a copy. Specifically, the issue is on this line:

const nextValue = value

Because value is an object (not a primitive like number or string), both value and nextValue now point to the same object in memory. So, when we update the nextValue object on the next line,

nextValue[target.name] = target.value

The original value object also changes.

In many cases, this kind of mutation can occur without serious side effects. In particular, if the component that holds the state is the only component that depends on the state, then mutating it will usually have no consequences. The problem there—and the reason that it's easier just to say "don't mutate state"—is that there's no way to enforce that rule. Some other developer (or Future You, after you've forgotten all about this) may refactor that component and break the unspoken rule.

Speaking of unspoken rules, remember the one from earlier about needing to memoize the defaultValue prop? It turns out that's the second piece to this mystery. Because each element in the array points to the same object, mutating any of them mutates all of them. Even creating a new copy of the array inside the ArrayField callbacks can't get around this because those copies are shallow. In other words, the copied array is a new instance, but each index of the new array points to the same object as the corresponding index of the original array.

The Fix

Let's review what we've found so far. Two simultaneous factors cause this bug: changing a field within the sub-form mutates the existing value for that form, and each element in the array of values points to the same original object. To fix it, we need to address at least one—but ideally both—of them.

First, we'll fix the mutation. We can use the spread operator to create a copy of the current value object. It's important to note that, like copying the array, this only makes a shallow copy. So if the value object has nested objects as properties, we'll need to remember to handle that specifically.

const changeValue = (event: React.FormEvent) => {
  const target = event.target as HTMLInputElement
  const nextValue = { ...value }
  nextValue[target.name as keyof User] = target.value
  onUpdate(index, nextValue)
}

We can simplify this a bit by combining the spread operator with the property update, using a computed property name:


```tsx {4,5}
const changeValue = (event: React.FormEvent) => {
  const target = event.target as HTMLInputElement
  onUpdate(index, {
    ...value,
    [target.name as keyof User]: target.value,
  })
}

This change alone will fix the bug, but it leaves us with another unspoken rule that every developer will need to know about whenever they update or change any of these form components. To ensure we fix this bug everywhere, we should also have each element in the value array point to a unique object. We can do that by changing the "append" and "reset" callbacks in ArrayField to create a copy of the default object.

const appendItem = () => {
  onChange([...value, { ...defaultValue }])
}

// ...snip: other callbacks stay the same

const resetIndex = React.useCallback(
  (index: number) => {
    onChange((value) => {
      const nextValue = [...value]
      nextValue[index] = { ...defaultValue }
      return nextValue
    })
  },
  [onChange, defaultValue]
)

You could even take this a step further by changing the defaultValue prop to a function, like getDefaultValue. That way, the function itself can be memoized, but the object returned could be a new instance each time. A change like this isn't necessary for every use case, but the advantage it has over the simple copy from above is that it also works for objects with nested objects as properties. For example, imagine we update UserForm with an address property, an object with all the usual address fields.

// ArrayField.tsx

import * as React from 'react'

type ArraySubFieldProps<T> = {
  label: string
  value: T
  index: number
  onUpdate: (index: number, value: T) => void
  onRemove: (index: number) => void
  onReset: (index: number) => void
}

type ArrayFieldProps<T> = {
  label: string
  itemLabel?: string
  component: React.ElementType<ArraySubFieldProps<T>>
  getDefaultValue: () => T
  value: T[]
  onChange: React.Dispatch<React.SetStateAction<T[]>>
}

function ArrayField<T>({
  label,
  itemLabel = 'Item',
  component: Component,
  getDefaultValue,
  value,
  onChange,
}: ArrayFieldProps<T>) {
  const appendItem = () => {
    onChange([...value, getDefaultValue()])
  }

  // ...snip: the other callbacks stay the same

  const resetIndex = React.useCallback(
    (index: number) => {
      onChange((value) => {
        const nextValue = [...value]
        nextValue[index] = getDefaultValue()
        return nextValue
      })
    },
    [onChange, defaultValue]
  )

  // ...snip: the returned JSX stays the same, too
}

// DemoForm.tsx

import * as React from 'react'
import type { User } from './UserForm'
import { UserForm } from './UserForm'

export function DemoForm() {
  const [users, setUsers] = React.useState<User[]>([])

  return (
    <div>
      <ArrayField
        label="Users"
        itemLabel="User"
        component={UserForm}
        getDefaultValue={React.useCallback(
          () => ({
            firstName: '',
            lastName: '',
            email: '',
            address: {
              street: '',
              city: '',
              state: '',
              zip: '',
            },
          }),
          []
        )}
        value={users}
        onChange={setUsers}
      />
    </div>
  )
}

If you found this post helpful and want to take a look at all of the code, I've created a sandbox with both the broken and fixed versions. And if you find yourself building something similar to the ArrayField component, I highly recommend checking out react-hook-form for managing the form state. This library includes a useFieldArray hook for exactly this use case.