3

So we decided to redo UI of our web application in React. Six months down the lane and we have a complete mess of components and reducers and thunks and actions and god knows what not.

We have multiple files named reducer.ts and each file is 3000-5000 lines long with thousands of reducer functions in each.

Same is the case with actions.ts files and then accessor.ts files. Interestingly, thunk.ts files are god files with lines approaching to millions (exaggerating, but you got the idea).

Then there is a few thousand lines long api.ts file that handles every possible call to the api server.

At the root level, we have a types.ts file that carries more than 30,000 lines in a single file with every typescript type defined in it.

I have been with small scale React apps for few years now but never had any exposure to this level of project size and messy code.

I am sure you guessed already that there are no Unit tests at all.

Primary problem is that the very promise that React provides of independent components that can be refactored and managed individually seems completely broken in this scenario.

Obviously there is no problem with react itself but I guess the way it has been used in our case is way off the mark. It takes hours to even locate the code for a very simple bug because everything is so deeply interconnected, and every functionality is scattered among at least six to ten files, it is often unclear where some change is happening.

My questions to the veteran react devs are as below:

  • Will it be a cardinal sin if I remove the redux and thunk and types and accessors and move the logic of each component inside that component? This way we won't have to jump six to ten files to search for the code of a single action.

  • Even if we have to keep the store, and reducers and accessors and api.ts, is it a religious practice to keep everything in the same files and make them god files? Can I at least create separate reducer or accessor or API-call or thunk files for each action/feature I am handling?

  • Is there any recommended practice for code organization when building large scale React apps?

I am from a strong unit testing background and I believe it is imperative to have clean, well separated and testable code for better maintenance.

Am I thinking in the wrong direction because in React world the way project is already done is the right way to do things?

Thanks for any clues.

  • 4
    "Primary problem is that the very promise that react provides of independent components that can be refactored and managed individually" Advice from having worked on projects that have suffered similarly: a different way to manage the same things does not solve the issue of bad management. Regardless of the tech stack/framework you use, it can always be misused. I refuse to believe that in your thousands upon thousands of lines of code, there's isn't a lot of duplication going on, which is exactly what bad management looks like for "individual" components. – Flater Jan 21 at 8:57
  • In the JS world it seems common to use tiny files, often with just a single function each. There's no need to have huge files with thousands of lines of code. Don't make your life unnecessarily hard. But yes, writing a React app that doesn't devolve into an unmaintainable mess is rather tricky. – amon Jan 21 at 9:22
  • @Flater: By "bad management" do you mean "bad architecture" or "bad design"? Management makes me think it is a "people problem." This feels like an engineering problem. – Greg Burghardt Jan 22 at 13:56
  • @GregBurghardt "Managing the development process/codebase badly", i.e. by developers, not by managers. – Flater Jan 22 at 13:58
3

As a relatively senior guy in my org, I just plainly reject PRs that have insane code structure, like 30k lines in a single file (I bet much of that is copy-paste).

My suggestion would be to talk to the team and make them aware of the best, or at least reasonably idiomatic, React practices, and then enforce them.

Make sure you actually can afford doing that, though. That is, make sure the management understands that the problem lowers your velocity, and increases the bug frequency drastically, so fixing it is worth the effort.

Of course fixing the mess in one fell swoop is already infeasible. I would make several tickets to fix the most egregious offenders in the code, then required the code improved every time it's touched (again, allocating time for that).

But education, and the buy-in form the team's engineers, comes first. People won't follow a practice in which they don't see merit. Explain the problems, explain possible solutions, explain the upsides. It may need to take several rounds, and certain patience.

In any case, what you describe is not a problem of React, but of the engineering culture, and should be fixed there.

| improve this answer | |
1

Thanks everyone for the insights. As pointed out by the comments and posted answers above, we had multiple problems (some of which were not even visible till I read these answers).

Although we are still in a long running refactor mode now, but for the sake of completion, I am posting here some hints for the reference. Hopefully it may help someone in the future.

First problem was communication. As suggested by @9000, Flater and Greg, it was mainly a people problem.

After I talked openly with the team, it turned out that I was the only one concerned with the bad architecture and messy code. Tight deadlines were another reason to keep anyone from thinking out of the box.

After a week of roaring meetings, we finally agreed upon the primary fact that there is a problem in our current coding practice and we need to address it.

The approach we have adopted, is as below.

  • as the app is already live and being used by over a thousand users, we can not go with breaking mode.
  • We started to use IntelliJ extract features to extract parts of code into smaller independent files
  • every time code is extracted to a smaller independent file/class, unit tests for that specific part are added.
  • we decided to go slow and steady. so refactoring is moving at a steady pace of one or two extractions per developer per day. Although it seems slow, but we have an estimation that with this pace, within three months, we should have at least 30-35% code base covered with unit tests and we will be in a much better shape than we are in now.
  • we have added an arbitrary review rule that any time we have a new class/file which is bigger than 100 lines, at least three reviewers should discuss and try to refactor it (if possible) to smaller units.

So far we are very happy with the results. Any new code being pushed is already in a better shape plus our old code has started to become more testable and more understandable.

| improve this answer | |
0

Are you the only one who is concerned about the code organization? Are you prepared to drive a discussion on coding standards with your team? What about management, have they been made aware of the risk of coming to a standstill because of tech debt?

In a first round, automated refactorings like extract function or move class to file are your friend. JetBrains have been good with this. I am sure there are others. Even in absence of tests, these should be relatively safe and make a good starting point to reduce tight coupling in code.

| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.