Solution to Design Challenge 006: Breaking Down a God Component

At the end of Design Challenge 006, the code had reached a point where it became hard to read and understand. It was accepting so many props—some of them unrelated—that controlled the behavior of the component in different ways. It was like a machine with too many dials and buttons, where you couldn’t clearly see its responsibilities until you read a massive manual first. And how would you even begin to test it before making changes?

Let’s take another look at the code before we proceed.

const ItemSelector = ({
  isLoading,
  hasError,
  errorMessage,
  customErrorState,
  customSkeleton,
  onSortGroupItem,
  items,
  //... many others
}: ItemSelectorProps) => {

  if (isLoading) {
    return customSkeleton || <ItemSelectorSkeleton />;
  }

  if (hasError) {
    if (featureToggle("patch-for-xyz")) {
      return <YetAnotherErrorState errorMessage={errorMessage} />;
    }

    return customErrorState || <DefaultErrorState />;
  }

  const sortedItems = onSortGroupItem(items);

  return <>{/* render sorted items */}</>;
};

I’ve skipped over some of the item rendering code, which is already a large part of the component. This includes logic to categorize items into groups (HR-related items, legal items, IT services, etc.), search for items by keyword, preview an item when a tile is selected, and so on.

A common strategy for handling such a "God component" is to group related props together. This is similar to what Martin Fowler refers to as “Data Clumps” in his book Refactoring.

Grouping related props?

For example, we could group all the props related to error handling and see how they affect the behavior of the component.

When there’s an error, we either render a customErrorState or a DefaultErrorState. To complicate matters further, in the specific case of the “patch-for-xyz” feature, we render yet another error component with an errorMessage.

if (hasError) {
  if (featureToggle("patch-for-xyz")) {
    return <YetAnotherErrorState errorMessage={errorMessage} />;
  }

  return customErrorState || <DefaultErrorState />;
}

Similarly, when isLoading is true, we either render the ItemSelectorSkeleton or a custom one (customSkeleton).

Should we do less?

But as we start grouping props, a new question arises: why is the ItemSelector handling loading and error states at all? These are separate concerns that don't relate to the core function of selecting and displaying items. Splitting out this logic could streamline the component, making it more efficient and easier to understand.