The Power of Simplifying Large Components in React
Tags: javascript, react, node, webdev
Published: 2020-01-04 15:21:12

Find me on [medium](https://medium.com/@jsmanifest)

Having large components _isn't always a bad thing_, but it's a good practice to take advantage of opportunities where you can simplify components further _especially when it provides additional benefits_.

When you have a large component, it _can_ become disadvantageous because the larger a component gets the more difficult it becomes to maintain and read over time.

Let's look at this component below and see reasons why it'd be better off simplifying it.

(This is code from a production app, so this is actually a _real world_ example)

The component `SidebarSection` below takes some props where `props.ids` is an array of item ids as strings and `props.items` is an object that maps sidebar items using each item's `id` as the key. It uses these props to render sidebar items:

```jsx
import React from 'react'
import { useSelector, useDispatch } from 'react-redux'
import List from '@material-ui/core/List'
import Divider from '@material-ui/core/Divider'
import ListSubheader from '@material-ui/core/ListSubheader'
import { EDIT_NOTEBOOK, DELETE_NOTEBOOK } from 'features/toplevel'
import { selectSelected } from 'features/sidebar'
import SidebarContext from './SidebarContext'
import SidebarItem from './SidebarItem'

function SidebarSection({ id: sectionId, ids, items, depth, expanded }) {
  const ctx = React.useContext(SidebarContext)
  const selectedId = useSelector(selectSelected)

  if (!ctx) return null

  return (
    <List dense={depth > 0} disablePadding>
      {ids.map((id: string, itemIndex: number) => {
        const key = `SidebarSection_${id}_item${itemIndex}`
        const item = items[id]

        switch (item.type) {
          case 'divider':
            return <Divider key={key} style={{ padding: 0, margin: 0 }} />
          case 'label':
            return (
              <ListSubheader
                key={key}
                style={{
                  transform: expanded ? undefined : 'scale(0.55)',
                  textOverflow: 'ellipsis',
                  overflow: 'hidden',
                  userSelect: 'none',
                }}
                disableGutters={!expanded}
              >
                {item.label}
              </ListSubheader>
            )
          case 'notebook': {
            // Called inside unserializeHoverControlsProps when iterating over each hover action
            const onHoverAction = (action: any) => {
              if (action.Icon) {
                const notebook = item.data
                if (notebook) {
                  action.onClick = ctx.createHoverControlsActionOnClick({
                    context:
                      action.name === 'edit'
                        ? EDIT_NOTEBOOK
                        : action.name === 'delete'
                        ? DELETE_NOTEBOOK
                        : '',
                    data:
                      action.name === 'edit'
                        ? item
                        : action.name === 'delete'
                        ? {
                            id: notebook.id,
                            title: notebook.info.title,
                            description: notebook.info.description,
                            isEncrypt: notebook.isEncrypt,
                            created_at: notebook.created_at,
                            modified_at: notebook.modified_at,
                          }
                        : null,
                  })
                }
              }
            }

            return (
              <SidebarItem
                key={key}
                sectionId={sectionId}
                depth={depth}
                item={ctx.unserializeItem(item, { onHoverAction })}
                isSelected={item.id === selectedId}
                {...ctx}
              />
            )
          }
          case 'static':
            return (
              <SidebarItem
                key={key}
                sectionId={sectionId}
                depth={depth}
                item={ctx.unserializeItem(item)}
                isSelected={item.id === selectedId}
                {...ctx}
              />
            )
          default:
            return null
        }
      })}
    </List>
  )
}
```

The component actually doesn't look _that_ bad, but if you think about it whenever we edit the component we'd have to understand _every_ line of code before we introduce changes because we don't know if changing something can break other parts of the component or not.

An example is the `onHoverAction` function that is created in the switch case. it's unnecessarily bloating our component, and depending on the implementation of `SidebarItem` it has potential to cause an _infinite loop_ because the reference to it is being re-created every time the component re-renders.

It's also making this whole component a little more sensitive to unit tests because we're delegating the `SidebarSection` component to be responsible for the implementation details of `onHoverAction`. In our unit tests, we have to be aware of the implementation details of `onHoverAction` when we're testing that the `SidebarSection` component behaves correctly which doesn't make much sense (this means watching out for things like syntax errors, since a typo inside the function can break the rendering of `SidebarSection` and we'd blame the component for doing a bad job)

We can simplify this by simply extracting it outside so we no longer have to put the blame on the component:

```jsx
function onHoverAction(item, createOnClick) {
  return (action) => {
    if (action.Icon) {
      const notebook = item.data
      if (notebook) {
        action.onClick = ctx.createHoverControlsActionOnClick({
          context:
            action.name === 'edit'
              ? EDIT_NOTEBOOK
              : action.name === 'delete'
              ? DELETE_NOTEBOOK
              : '',
          data:
            action.name === 'edit'
              ? item
              : action.name === 'delete'
              ? {
                  id: notebook.id,
                  title: notebook.info.title,
                  description: notebook.info.description,
                  isEncrypt: notebook.isEncrypt,
                  created_at: notebook.created_at,
                  modified_at: notebook.modified_at,
                }
              : null,
        })
      }
    }
  }
}

function SidebarSection({ id: sectionId, ids, items, depth, expanded }) {
  const ctx = React.useContext(SidebarContext)
  const selectedId = useSelector(selectSelected)

  if (!ctx) return null

  return (
    <List dense={depth > 0} disablePadding>
      {ids.map((id: string, itemIndex: number) => {
        const key = `SidebarSection_${id}_item${itemIndex}`
        const item = items[id]

        switch (item.type) {
          case 'divider':
            return <Divider key={key} style={{ padding: 0, margin: 0 }} />
          case 'label':
            return (
              <ListSubheader
                key={key}
                style={{
                  transform: expanded ? undefined : 'scale(0.55)',
                  textOverflow: 'ellipsis',
                  overflow: 'hidden',
                  userSelect: 'none',
                }}
                disableGutters={!expanded}
              >
                {item.label}
              </ListSubheader>
            )
          case 'notebook': {
            return (
              <SidebarItem
                key={key}
                sectionId={sectionId}
                depth={depth}
                item={ctx.unserializeItem(item, {
                  onHoverAction: onHoverAction(
                    item,
                    ctx.createHoverControlsActionOnClick,
                  ),
                })}
                isSelected={item.id === selectedId}
                {...ctx}
              />
            )
          }
          case 'static':
            return (
              <SidebarItem
                key={key}
                sectionId={sectionId}
                depth={depth}
                item={ctx.unserializeItem(item)}
                isSelected={item.id === selectedId}
                {...ctx}
              />
            )
          default:
            return null
        }
      })}
    </List>
  )
}
```

All we did was move the function to another place, and it already gives us huge benefits with hardly any extra effort:

1. The reference to the function will stay the same.
2. The `SidebarSection` can now live a peaceful life as it no longer needs to worry about implementing `onHoverAction` correctly. All it needs to do is pass the arguments that `onHoverAction` expects.
3. We can now unit test `onHoverAction` separately because it's available as an export. Want to see if this is working expectedly? Simply import it, pass in the three parameters and see what it returns.
4. `SidebarSection` becomes easier to read and maintain.

That's actually not all we can do to simplify it. We have another opportunity to simplify the component even further. There's duplicated code in these two switch blocks:

```jsx
case 'notebook':
  return (
    <SidebarItem
      key={key}
      sectionId={sectionId}
      depth={depth}
      item={ctx.unserializeItem(item, {
        onHoverAction: onHoverAction(
          action,
          item,
          ctx.createHoverControlsActionOnClick,
        ),
      })}
      isSelected={item.id === selectedId}
      {...ctx}
    />
  )
case 'static':
  return (
    <SidebarItem
      key={key}
      sectionId={sectionId}
      depth={depth}
      item={ctx.unserializeItem(item)}
      isSelected={item.id === selectedId}
      {...ctx}
    />
  )
```

It actually may not become much of an issue to just leave it the way it is. However, I'm sure that any developer who reads this code will be obligated to read each prop line by line just to be 100% sure themselves that they aren't all that different.

After all, ideally we'd want to believe that there are important reasons for similar looking code get separated, so why in the world were these separated? In our case there wasn't really much of a good reason, so it's a good idea to simplify this so that future developers won't get caught in this awkward scenario when they're trying to debug this component.

We can simplify this by simply doing this:

```jsx
case 'notebook':
case 'static':
  return (
    <SidebarItem
      key={key}
      sectionId={sectionId}
      depth={depth}
      item={ctx.unserializeItem(item, item.type === 'notebook' ? {
        onHoverAction: onHoverAction(
          action,
          item,
          ctx.createHoverControlsActionOnClick,
        ),
      } : undefined)}
      isSelected={item.id === selectedId}
      {...ctx}
    />
  )

```

Simplying this provided a couple of important benefits:

1. We eliminated duplicate code.
2. It is now easier to read since we only need to look at one "copy" of the code.
3. Self documenting code (it basically tells us that items with type _"notebook"_ and _"static"_ are almost exactly the same and there's not much need to worry about their differences besides that items with type `'notebook'` can be clickable and `'static'` is not)

## When simplifying backfires with overthinking

Now there is something else that we can possibly "simplify". Even though our switch cases became a little shorter, it looks a little ugly to look at. This is how our `SidebarSection` component looks like now with the simplification changes applied:

```jsx
function SidebarSection({ id: sectionId, ids, items, depth, expanded }) {
  const ctx = React.useContext(SidebarContext)
  const selectedId = useSelector(selectSelected)

  if (!ctx) return null

  return (
    <List dense={depth > 0} disablePadding>
      {ids.map((id: string, itemIndex: number) => {
        const key = `SidebarSection_${id}_item${itemIndex}`
        const item = items[id]

        switch (item.type) {
          case 'divider':
            return <Divider key={key} style={{ padding: 0, margin: 0 }} />
          case 'label':
            return (
              <ListSubheader
                key={key}
                style={{
                  transform: expanded ? undefined : 'scale(0.55)',
                  textOverflow: 'ellipsis',
                  overflow: 'hidden',
                  userSelect: 'none',
                }}
                disableGutters={!expanded}
              >
                {item.label}
              </ListSubheader>
            )
          case 'notebook':
          case 'static':
            return (
              <SidebarItem
                key={key}
                sectionId={sectionId}
                depth={depth}
                item={ctx.unserializeItem(
                  item,
                  item.type === 'notebook'
                    ? {
                        onHoverAction: onHoverAction(
                          action,
                          item,
                          ctx.createHoverControlsActionOnClick,
                        ),
                      }
                    : undefined,
                )}
                isSelected={item.id === selectedId}
                {...ctx}
              />
            )

          default:
            return null
        }
      })}
    </List>
  )
}
```

One issue we might come up with here is that we are giving the render block of each item a little too much responsibility, making it responsible for passing the right props to the right components.

Thinking that way it might be better to rewrite it this way instead:

```jsx
function getProps({ item, expanded, sectionId, selectedId, depth, ctx }) {
  switch (item.type) {
    case 'divider':
      return { style: { padding: 0, margin: 0 } }
    case 'label':
      return {
        style: {
          transform: expanded ? undefined : 'scale(0.55)',
          textOverflow: 'ellipsis',
          overflow: 'hidden',
          userSelect: 'none',
        },
        disableGutters: !expanded,
      }
    case 'notebook':
    case 'static':
      return {
        sectionId,
        depth,
        item: ctx.unserializeItem(
          item,
          item.type === 'notebook'
            ? {
                onHoverAction: onHoverAction(
                  item,
                  ctx.createHoverControlsActionOnClick,
                ),
              }
            : undefined,
        ),
        isSelected: item.id === selectedId,
        ...ctx,
      }
    default:
      return undefined
  }
}

function SidebarSection({ id: sectionId, ids, items, depth, expanded }) {
  const ctx = React.useContext(SidebarContext)
  const selectedId = useSelector(selectSelected)

  if (!ctx) return null

  return (
    <List dense={depth > 0} disablePadding>
      {ids.map((id: string, itemIndex: number) => {
        const key = `SidebarSection_${id}_item${itemIndex}`
        const item = items[id]

        let Component

        if (item.type === 'divider') {
          Component = Divider
        } else if (item.type === 'label') {
          Component = ListSubheader
        } else if (['notebook', 'static'].includes(item.type)) {
          Component = SidebarItem
        } else {
          return null
        }

        return (
          <Component
            key={key}
            {..getProps(
              item,
              expanded,
              sectionId,
              selectedId,
              depth,
              ctx
            })}
          />
        )
      })}
    </List>
  )
}
```

Now we further simplified `SidebarSection` to only be responsibility for calling `getProps` to provide the associated props and assigning the correct `Component` based on `item.type`. We can now unit test `getProps` to make sure that they are returning the correct props according to `item.type`.

Was this a good attempt to simplify our react code? Let's see the benefits gained vs the downsides introduced:

Benefits:

1. `SidebarSection` reduced its responsibilities.
2. `SidebarSection` became smaller.
3. We can clearly see what props are being injected to which component.
4. We now don't have to pass in `key={key}` four different times and instead just pass it like `<Component key={key}`

Downsides:

1. `SidebarSection` becomes smaller, but the file becomes larger.
2. One "entity" (everything was inside `SidebarSection`) became three "entities" (now separated to `SidebarSection`, `onHoverAction`, `getProps`)
3. Stressing out our mouse more by scrolling top to bottom to get through the whole thing

## So was it worth it?

In my honest opinion, if it takes too long to do the last part then it probably isn't worth it. The moral of the story is, it's _definitely worth it to simplify code where it doesn't take much effort but still provides more multiple benefits in the outcome_.

So in the case of our article, I support the first two simplification attempts in this post while I'm a little undecided on the third one.

However, we've now seen the power of simplifying large components in react.

## Conclusion

And that concludes the end of this post! I hope you found this to be valuable and look out for more in the future!

Find me on [medium](https://medium.com/@jsmanifest)