React: bad habits

#react #webdev #typescript

Bad habits, we all have them! In this article, I will confess some of my own coding bad habits and bad habits I have encountered in React code over the past years, working as a React dev.

1. Create derived state using a side effect

This is something that happens a lot. Usually because initially some values were fetched from an api and it seemed logical to add up the values from the api call in the same useEffect:

function ShoppingCart({ items }: { items: Array<{ name:string, price: number }> }){
  const [totalPrice, setTotalPrice] = useState(0)

  useEffect(() => {
    // suppose there was initially an API call here, which fetched the items
    const sum = items.reduce((current, item) => current += item.price, 0)
    setTotalPrice(sum)
  }, [items])

  return <ul>
    {items.map((item, index) => <li key={index}>{name} - {price}</li>
    <li>Total: {totalPrice}</li>
  </ul>
}

However, setting derived state inside a useEffect hook should be avoided at all times! Instead, what we want is the useMemo hook:

function ShoppingCart({ items }: { items: Array<{ name:string, price: number }> }){
  const totalPrice = useMemo(() => items.reduce((current, item) => current += item.price, 0), [items])

  return <ul>
    {items.map((item, index) => <li key={index}>{name} - {price}</li>
    <li>Total: {totalPrice}</li>
  </ul>
}

This saves us an extra rerender that would be triggered if we change the state inside a side-effect that is based on some other state. Saving one rerender does not seem like a big deal, but it adds up quickly if the component gets nested inside another component that also sets derived state in a useEffect etc...

2. Managing multiple state properties

Creating forms is something we do a lot and it usually looks something like this:

function ContactForm(){
  const [email, setEmail] = useState('')
  const [name, setName] = useState('')

  const handleSubmit = useCallback(() => {
    // submit data...
  }, [])

  return <form onSubmit={handleSubmit}>
    <input value={email} onChange={e => setEmail(e.target.value)} />
    <input value={name} onChange={e => setName(e.target.value)} />
    <button type="submit">submit</button>
  </form>
}

And we all know what happens when someone asks us if we can add another field to the form... we add another useState and another input and call it a day. I've done this myself, but the question becomes, when do we stop? Because obviously, it is really dirty to have 20 lines of useState hooks in one component. If a component has more than one state property, use a reducer instead! I know reducers have this bad reputation of being complicated and redux-y, but it is actually one of the simplest ways to manage several state properties in a single component:

const initialState = {
  email: '',
  name: ''
}

function formReducer(prevState: typeof initialState, nextState: Partial<typeof initialState>){
  // no need for redux-like switch statements here!
  // just spread the next state onto the previous one, and we're done.
  return { ...prevState, ...nextState }
}

function ContactForm(){
  const [{ email, name }, setValue] = useReducer(formReducer, initialState)

  const handleSubmit = useCallback(() => {
    // submit data...
  }, [])

  return <form onSubmit={handleSubmit}>
    <input value={email} onChange={e => setValue({ email: e.target.value })} />
    <input value={name} onChange={e => setValue({ name: e.target.value })} />
    <button type="submit">submit</button>
  </form>
}

3. Creating black box context providers

Suppose we have some app context that we use to share some settings with the components in our app. This is a pretty common pattern and normally there's nothing wrong with it. However, there are some dangerous pitfalls here! Take this provider for example:

const initialState = {
  darkMode: true
}

function settingsReducer(prevState: typeof initialState, nextState: Partial<typeof initialState>){
  return { ...prevState, ...nextState }
}

const AppSettings = React.createContext({
  settings: initialState,
  changeSettings: (() => {}) as (settings: Partial<typeof initialState>) => void
})

function useAppSettings(){
  return useContext(AppSettings)
}

function AppSettingsProvider({ children }: React.PropsWithChildren<unknown>) {
  const [settings, changeSettings] = useReducer(settingsReducer, initialState)

  useEffect(() => {
    // some side effect, for example:
    settings.darkMode && import ('../styles/darkMode.css')
  }, [settings.darkMode])

  return <AppSettings.Provider value={{ settings, changeSettings }}>
    {children}
  </AppSettings.Provider>
}

It might seem logical, but when we do this, we need to realize that other developers working on our application are not going to realize this side effect exists there. In general, side effects in providers should be avoided. A provider is a blackbox except for the values that we expose from it (the context value). What we can do instead (in this case), is create a new component with an explicit name like DarkModeThemeLoader which only handles the loading of the darkMode styles, so other developers working on our app do not have to guess where this logic lives:

function DarkModeThemeLoader(){
  const { settings } = useAppSettings()

  useEffect(() => {
    settings.darkMode && import ('../styles/darkMode.css')
  }, [settings.darkMode])

  return null
}

// usage example:
function App(){
  return <AppSettingsProvider>
    <DarkModeThemeLoader />
    ... other content ...
  </AppSettingsProvider>
}

Top comments (7)

Corners 2 Wall • Mar 21 '23

I don't understand first point. We can install eslint plugin for passing necessary dependencies or you are doing something else. I can't grasp this. Could you explain clearly please? Also premature optimization is the root of all evil. Use everywhere useMemo and useCallback not bad but sometimes it's excess.

Third point, in short words we violated single responsibility. Why load something in providers?

Rense Bakker • Mar 22 '23

Thanks for replying! Yes, everyone should be using exhaustive deps eslint rule! However, that's not the point I'm trying to make. I'm trying to give an example of a pattern that I see a lot, where people creating derived state using a side effect. In other words, they're responding to a state change in the component, by updating another piece of state, thus causing another rerender of the component. Basically they're chaining updates to the component. By using the useMemo hook, we avoid the extra rerender. I agree we should not prematurely waste hours trying to optimize our application, however, in my opinion, proper usage of react hooks doesn't fall under this category of premature optimization.

On the 3rd point, that's a good point yes, the code example also violates the single responsibility principle.