DEV Community

Mohammad Faisal
Mohammad Faisal

Posted on • Originally published at mohammadfaisal.dev

How do you Refactor a 2,700-Line React Component?

To read more articles like this, visit my blog

We are always so excited about the new shiny things that come out every week in the programming world. New ways to structure components, new techniques to reduce those two lines of code, and so on.

But in the real world, things are not so shiny. We often have to deal with codebases that have been evolving for many years, and many developers have left their mark on the components' different parts.

Our story is about a component that has 2700 lines of code. Let’s try to explain how things went south and how we can do better.

Background

I worked in a company with a fleet management tool dashboard showing vehicles roaming around the city in real-time.

The dashboard component is the hero of our story today. It has many functionalities. Nothing fancy, though.

  • A map that shows the vehicle markers

  • A way to search through the vehicles

  • A popup that shows vehicle details when clicked

  • A list of vehicles at the bottom

  • Some filtering options.

Sure, there are several features that this single component is responsible for displaying. But is it that much to contain 2700 lines of code?

The Component

Obviously, I will be stupid to paste 2700 lines of code into this article. (Also, it will be illegal :P ) But let me show you the general structure of the component.

// import statements -> 85 lines 
import React from 'react';
import styled from 'styled-components'
...
...

--------------

//constant declaration -> 15 lines
const MAX_LOOP= 40;
const FAILURE_MESSAGE = "Something Brutal Happened!"
...
...

--------------

//styled component declaration -> 580 lines!!!!!
const SomeStyle = styled.div`
 display:flex;
 flex-direction: column;
`
...
...

---------------

//Helper components -> 200 lines
const MapPortal = () => {

  return <div>
    // .. some placeholder component
  </div>
}
...
...


---------------

// Props and State Type Declaration -> 70 lines

type Props = {
  vin: string;
  .....
}

type State = {
  mapVisibility: boolean;
  ....
}

---------------

// The actual component with all it's helper methods -> 1700 lines!!!!!!!!!!!!!!!!!!

export class TheActualComponent extends React.Component<Props , State>{

  constructor(props: Props){
    // ...
  }

  componentDidMount() {
    // ... And other lifecycle methods
  }

  randomHelperMethod(){
    // These functions extracts some logic into a separate function
  }

  onClickHandlers() {
    // Lots of them!
  }

  render(){
    return <div>
       // {Something worth of it?}
    </div> 
  }

}


// redux+type declarations -> 200 lines

export const mapStateToProps = {
  // .. lots of them!
}
export const mapDisptachToProps = {
  // .. lots of them
}

export Component
Enter fullscreen mode Exit fullscreen mode

Okay Now Tell Me What’s Wrong Here

To be 100% honest? Almost everything. Let me explain those.

1. Constant Declaration

This is such an obvious one. Unfortunately, I have seen many examples of it throughout many companies and many components.

You should Keep the Constants in a Separate File

It doesn’t matter if they are being re-used or not. It’s still better to store them in a separate File. Because down the line, someone else would create a separate constant with the same value, eventually creating confusion.

I do it this way.

// for related constants
export DeclaredConstants {
  SOME_CONSTANT_1 = "SomethingSpecial1";
  SOME_CONSTANT_2 = "SomethingSpecial2"
}

// or for single constants

export const SOME_CONSTANT_3 = "SomethingSpecial3"
Enter fullscreen mode Exit fullscreen mode

And then import the inside my component and use them.

import { DeclaredConstants } from "ConstantDeclaration.ts"

console.log(DeclaredConstants.SOME_CONSTANT_1)
Enter fullscreen mode Exit fullscreen mode

This particular component isn’t a big deal, but best practices are best practices.

2. Styles and Helper Methods

I think it’s a very common mistake and (sometimes allowable) for smaller components to put the styles and helper methods in the same file.

If your component is only 30–50 lines of code, then it can make sense to keep the styles and helper methods in the same file.

But having 580 lines of style declaration doesn’t make sense in any scenario. Because not very often you would need to touch these styles.

What I do is follow the following folder structure to keep things organized.

-component-name 
  |-- __tests__
  |-- styles.ts
  |-- helpers.ts
  |-- index.ts
Enter fullscreen mode Exit fullscreen mode

The responsibility of the files is clear from the name of the files themselves. It’s so simple to simply split our massive component to 1/3rd of its current size just by putting things where they should be!

An Example

export const SomeStyle = styled.div`
  display: flex;
`
Enter fullscreen mode Exit fullscreen mode

If you are using Raw CSS or SCSS, then you probably don’t make this mistake, but the projects using styled-components or material-ui mostly follow this bad practice.

Once someone started it. It became the standard practice.

So don’t fall into this trap. Create a separate file for styles ahead of time that can save your component in the future.

3. Dumb Components

There are two types of components.

  • Dumb components -> only act as a container or view

  • Intelligent components -> Shows something based on some logic

Now there is no reason to put two components in the same file. It directly violates the Single Responsibility Principle.

Every class or component should do one thing and one thing only

Sometimes we get lazy (Myself included) and put simple container components into the actual component. But what do you think when the next developer comes?

Does he take out the smaller component into its own file?

Umm… Probably not. So after 4–5 years, you now have 200 lines of helper dumb components that can easily be extracted from separate files.

4. Class Component

Not sure if you noticed, but this Massive component is using ClassComponent. I’m sure you know the reason. It was written at a time when functional components were not that common.

But nowadays, using functional components make more sense because

  • They are easy to maintain

  • Less code

  • More performant? (Arguably)

But even I wouldn’t try to convert it into a functional component at this stage. We need to refactor many things before converting this into a functional component.

Let’s see how much we could gain.

Let me show you an estimation of how much we can improve without even understanding what this component is doing.

We can just export the constants to constants.ts , Styles into the styles.ts and helper methods into the helpers.ts file.

Styles                    =  600
Constants(Types+Others)   =  100
Helper Methods.           =  600 (roughly)
Dumb Components           =  200
---------------------------------
Total Gain                =  1400 lines!
Enter fullscreen mode Exit fullscreen mode

This is a work of two hours maximum. All we need to do is.

Putting things into appropriate files and import them

We can reduce a component from 2700 to 1300 lines!

Some might say that it’s still a lot. But hey!!! One step at a time, right?

Can we do better?

Yes, of course. When we look into the internal logic, we can do even better.

  • Break the actual components and re-usable parts into even smaller components

  • Using Functional component

  • Taking advantage of hooks

  • Using functional redux

And so on….. But that is a story for another day.

Show me the Good Parts.

Obviously, This component has lots of problems but some good things.

Typescript

Although the Type declarations add up to around 200 lines in total, they are worth it. Especially without Typescript, it would be impossible to maintain this component.

Extracting logic

Some of the dumb logic is extracted out of the view logic itself. For example, showing a message based on the vehicle status.

showVehicleStatusMessage (status: VehicleStatus){
  switch(status): {
    case SOME_STATUS: 
      return "Some Message"
    case SOME_OTHER_STATUS: 
      return "Some Other Message"
  }
}
Enter fullscreen mode Exit fullscreen mode

It’s better to have them in a separate function like this instead of writing the logic into the view. Which can look something like this…

return (
  {vehicleStatus ===  SOME_STATUS && <div> {"Some Message"} </div> 
)
Enter fullscreen mode Exit fullscreen mode

So it’s not all bad; some developers tried to do things correctly. At the end of the day, software development is a team effort.

What did I learn?

The most important takeaway for me is to understand the importance of following the best practices.

Best practices are there for a reason

The impact of following best practices may not be evident on the first day, but if you don’t follow those, you will feel the pain someday!

Conclusion

I hope you learned something new today. Have a great day!

Have something to say? Get in touch with me via LinkedIn or Personal Website

Top comments (1)

Collapse
 
eabuzaid profile image
Eyad Abu-Zaid

Absolutely amazing and relevant article! I deal with this all the time when I'm working on legacy code.

It's fascinating really how much time and effort you would save if you just apply the simplest of principles and best practices such as separation of concerns and modularity.

I don't think it's just laziness though. It's partly laziness but also the fact that developers have so many requirements that once they get something working they think "good, I'll just optimize it later" and move on to the next thing.

The "broken window" analogy in the book Clean Code best summarizes that thought process in my opinion.

Again, thanks for the post!