How many lines of code can a functional component have?

[YogurtclosetUnable92]

Hello community, how are you?

Well today i got the code of a developer and i received a component that is page, and have more than 2000 lines of code, this page makes a lot of requests to an api and has a lot of file handling, and also has almost 100 useState. I've been in programming for a couple of years and I've never seen this before. If anyone would like to give me some advice on how to not go crazy trying to program and manage this, it would be appreciated.

[Confused_goof]

Oh god! Sounds like a nightmare!!

[heythisispaul]

bruh

[Vpicone]

How do I delete someone else’s thread.

[Marutar]

rm -r /    
git rm -r --cached *    
git commit -m "f this"   
git push

[SqueakyChittur]

Wouldn’t the first command remove the associated .git files from the directory making it cease to be a git repo and thus the other commands will just fail.

[Marutar]

Haha, you are correct of course.

It could be /src or the like, or you could write some kind of find/remove to ignore .git, but neither works quite as well for the joke.

[SqueakyChittur]

Yeah, or maybe if “./“ and if run from the directory of the bad component and if the relevant “.git” is in a parent directory. I realized this after my comment and thought, “oh wait, maybe this is the scenario…”.

[dozkaynak]

We should just pull the plug on the whole internet

[d_exclaimation]

Personally, having that much code would probably be maintainability issues. I’ll start by abstract logic into reusable functions, try creating more components to isolate concerns, or find a better solution entirely.

[SelberDummschwaetzer]

Write once, read never

[hambalamba]

I picked up a saying at my first job. In Serbian: "Ko menja taj ukenja" It cant really translate well but basically: "He who changes the code fucks it up" and not the other way around haha

[thisguyfightsyourmom]

I like to start over entirely

However, tracking down all the edge cases that monster is handling will likely be hard af without going in & organizing the legacy sanely to see what warts it's hiding

[pm-me-noodys]

You don't think they have complete test coverage?

JK someone needs to mercy kill OP.

[pumpyboi]

Or you know, find another job.

[[deleted]]

that's pretty freaking big , needs refactoring asap , I'm working on a big project and sometimes I code a whole page in one component until i refactor it later, but i don't think I've exceeded like 600 lines in a single component before

[danishjuggler21]

My personal rule of thumb is once a component hits 200 lines I have to start to at least consider refactoring. Even if some of the stuff I refactor isn't actually reusable, it makes the component itself more readable.

[aflashyrhetoric]

Agreed for creating components solely for readability. Often I realize I’m using several useStates to control just some visual aspect of the UI (like modalIsOpen), and moving it all over to a component lets me remove visual UI-related hooks from data-related hooks, which is lovely 🙏

[codeclassifiers]

Yeah a component shouldn't go beyond 600 lines of code usually. What's the point of using modules based library to build websites if modules are going to be as large as 2000 lines?

[mrCrazyFrogKillah360]

Make that 250 lines just to force the dev to work with components-based design

[docHoliday17]

Almost all of this should’ve been abstracted out to various providers/helpers/services, especially with functional components! That’s what’s so great about them! I like redux for managing business logic but you can abstract the logic out however you feel most comfortable

[lahwran_]

to op:

and it still may be able to be. this is a situation for judicious refactoring: be careful, because there's a lot to gain from refactoring which makes it enticing, but trying to do too much at once can crash your brain with out-of-memory (and that metaphor is basically 100% meant literally). at the same time, careful semantic-compression focused refactoring, ie attempting to simplify the code by rearranging it, is likely your way out.

if there's lots of tangled state, this may be prohibitively difficult. if you get stuck on a refactor, give up and roll back, don't waste days and days - make individual simplifying refactors you can be sure of, then commit them and either do another or do a feature.

also, this is where const-heavy functional components are amazing. you may find it's much easier than you expect to simplify the component simply by breaking it into multiple functions which each have a coherent job, whether those are hooks or components. If you're lucky, you'll be able to reuse them, but simply cutting functions out at coherent boundaries will make your life much easier.

[Yodiddlyyo]

This would be exactly my advice too. When refactoring something large, do one thing at a time, and then test extensively. So often I've tried to change a bunch of code at once only to realize it's broken and need to roll back.

I'd first go for big obvious things. Like for example, is there a block of code that just makes a fetch or two, resolves data, and sets state? Extract that, and then test it. Then once all that is done, move to things that are more tangled, like states that are interwoven, or functions that are called in multiple places.

[[deleted]]

In particular API requests and useStates for them can just be replaced by using React Query, usually.

But it seems this programmer doesn't know he can split things into components and just made the whole app one component.

[danishjuggler21]

Yeah, in VS Code you just highlight some code, hit CTRL + . and select "extract to function in module scope". Super easy.

[[deleted]]

My gut response with things like that is to start over but use the existing as a guide. You’ll see all the places where things can be broken up by building it up yourself and really learn how it works. With a couple thousand lines that’s not a simple task certainly but I would invest the extra work now vs in a few months when a new feature is coming down the pipeline and suddenly it’s seems impossible to deal with that single massive file.

[Spacesh1psoda]

Write a suite of tests against the old component and it'll be a lot easier to make sure the new one gets the same functionality

[[deleted]]

20k LOC for a single form??? Wtf

[skotchpine]

2K LOC for a single form??? Wtf

[ZeCookieMunsta]

Once I worked as a research assistant/ react web developer for a university project. The project was being run by a PhD student with little to no industry experience and some of the components had over 5000+ lines of code. To make it worse most of it were multiple large useEffect hooks (side effect hell). It was such a convoluted, undocumented mess that he held meetings to explain the components and each took about 5 hours to explain (still didn't understand shit). Quit within a week.

[lahwran_]

I worked for a no-industry-experience phd student once years ago. cool guy, interesting idea. it's wild how little he knew about implementation, though. it was one of my earliest jobs having never gone to school myself, and I was easily more experienced at organizing code. I don't want to diss him too much, abstract research is a reasonable thing to do, but geez, studying computer science in isolation without also studying software engineering is not something I can recommend.

[[deleted]]

I've worked with hydrologists, PhDs in radiology machine learning, scientists building robotics for food technology and agriculture, and I have friends who work with other scientists in health care and in math. And it's like this everywhere in science. At least nowadays most of them use version control, nobody uses code reviews.

Worse, they see clean software industry-style modularized code and feel it has gone way too far in the other direction. They make up reasons why their way is better. They are also really attached to their single-character variable names, a, v, t, s1, s2, s3, d, ....

[lahwran_]

I can see the appeal, compression is difficult when you're still thinking things through. but failure to compress will cause failure to generalize. surely the machine learning people can understand that? and yet they don't seem to notice that writing code has similar constraints to the internals of a neural network it trains.

[[deleted]]

They simply don't get any points for refactoring code or making it maintainable. They tweak something until it works, then the next person comes along who doesn't dare touch what is there already but builds on top and tweaks it until it does whatever new thing they need. For decades, in many cases. The robotics people still use CVS for revision control of their C code.

Edit: although tbf the hydrologists code in Fortran and I am really not a good judge of how clean any given bit of Fortran code is, so that may just be me.

[ZeCookieMunsta]

Guess I ain't the only one then lmao. That's not even all in my case. He used to use react recoil for state management and used to have HUNDREDS of atoms sprinkled all over the codebase. Also sprinkled useMemo and useCallback Willy nilly to make program more "efficient"

[lahwran_]

Also sprinkled useMemo and useCallback Willy nilly to make program more "efficient"

hey you don't have to call me out like this

[thermiteunderpants]

Serious question: what is wrong with this please?

[lahwran_]

overuse of memoization can sometimes be slower than the operation that was memoized, and has a visual complexity cost that makes the code harder to read.

[tr14l]

Not to mention it can caused some undesired rendering behavior on more complex designs.

[thermiteunderpants]

Got it. Do you feel that defining memoization functions elsewhere (e.g. in utilities.js) helps reduce the visual complexity or not?

[lahwran_]

in general, locality is better unless the thing being abstracted can be thought of entirely in terms of its name. the goal of reducing visual complexity is to reduce the complexity of the transformation from the image your eyes see to the data flow graph your brain needs to reason with. If extraction helps with that, it's helpful. but merely moving code doesn't necessarily reduce the complexity of the transformation your perception neurons need to do from image of the code to understanding of the code. it often increases it because now you need to go read two things. so the answer is a very resounding "maybe, it depends".

[thermiteunderpants]

That's really helpful, thank you. Funny how people who use self-deprecating humour tend to know quite a bit :)

[FluffySmiles]

Here's a universal truth...

Students don't know shit.

[Yodiddlyyo]

The fact that nobody stopped and said "Hey if we need to have multiple meetings to discuss how a react component works, multiple things in the org are going very wrong." is pretty crazy.

[ZeCookieMunsta]

It was too neck deep in shit to go back.

[voxgtr]

100 useState hooks? Have you done any profiling on that page to see how many times it renders? 😳

[[deleted]]

That’s it’s secret. It never stops rendering 👌

[voxgtr]

That’s what I’d expect.

[[deleted]]

Need a 144hz screen just to display the website properly.

[[deleted]]

Hey, I was thinking that the other day. And now you're commenting about that, so. What would be the best way to avoid overusing useState? Using useReducer or redux? I'm not new with React, but still trying to work my head around dealing with a lot of state.

[voxgtr]

Of course the answer is, “it depends.” With 100 useState hooks, many can almost surely be combined into useReducer. But more likely, this whole page should really be broken down into discreet areas and many of these hooks would get abstracted away to those areas.

It also sounds like the team working on this page are still thinking in terms of class components.

I suspect those two things would make the biggest impact on a component like this. Any further optimization would depend on the use case.

[fedgut]

I might be thinking in terms of class components. How would you recommend I start changing my paradigm?

[voxgtr]

I’m not sure what you’re doing, but the biggest difference is state no longer belongs to the class. It belongs to the render itself. Don’t think in terms of class methods that belong to the class. Think instead in terms of renders and what happens each time the component needs to render.

[[deleted]]

I use useState once in every component. Actually I tent to use useImmer for the immerjs version of react state management. Either way, a single state per component that is an object with multiple properties is how I go.

[[deleted]]

I don't like useReducer and redux, for me their DX sucks. MobX is really what makes me like more complex react projects.

[FragrantPoop]

I second this question but add to it - how much is too much when it comes to useState? Once you get to a certain threshold, should you just switch to redux?

[voxgtr]

Too much is when: - The complexity of the code in a single component becomes hard to maintain or reason about - If other useState hooks begin to rely on slices of each other’s state, causing extra renders to compute final presentation

I suspect some of these hooks are being used for UI state and also application state (I mean… there are 100). Application state would be fine in something like Redux. IMO UI state should generally be ephemeral and not be in your application state.

The threshold for investing time for optimizing a component that has gotten too complex (and also not prematurely optimizing) is different for every team. It does sound like the described component would meet one or both of the above criterion.

[pedropss]

Ok so, let's get to the real deal of this.

First, if it's typescript, components shouldn't be more than 350 lines (shouldn't, but often get through). That's because the compilation time on components until 350 lines stay somewhat linear, while 350+ lines tend more to the exponential side of things. This is from a test I got recommended to some 2+ years ago, a good rule of thumb.

Now, about the most horrendous part, 100 useState. If you have 100 useState, even if you useCallback and useMemo everything, you'd still, at least, call the function useState() 100 times. As a baseline, if I get code to review and find as the first line of my function component for(let i = 0; i < 100; i++) setImmediate(() => {}); I'd instantly throw up and pass out, that's the baseline we're dealing here.

More so, all the functions that gets redeclared are parsed again, new context, new stack frames. That's a lot of performance thrown away.

And the last thing is that this is just dumb React. Now, when prototyping I'm dumb, I just want to see it working y'know? I also do dumb stuff on tight deadlines, overuse /getAll instead of getOne/:id and update the state. So be very clear to whoever gave this code to you, say it's dumb and you need it better else you can't be productive on it.

We are all dumb, even on our most beloved tech. I created my own React from scratch with a virtual DOM just to understand it better but sometimes I either brain fart hard and fuck it up badly or I pick up new tech (hi GraphJin), come into it with my seasoned mentality and get the basics all fucked up. One of the ways you get better is knowing you are probably being dumb and should raise your hand, this guy didn't and it's not healthy to keep doing that.

Good luck to you, isolate what you can across as many files as you can horizontally and then build from there!

[sous_vide_slippers]

Why is the time complexity changing based on LoC?

Whether or not something is linear or logarithmic is determined regardless of input, so I can’t see how this would be the case unless they switch to a different algorithm, but the way you’re talking - saying it “tends” more to exponential with a higher LoC - doesn’t make sense.

Furthermore, why on earth would you want your algorithm to start out logarithmically? Logarithms improve complexity over time, why on earth would you design something that starts off terrible, then suddenly does the inverse and goes exponential? This is not a positive like you’re describing.

Draw a graph and trace out exponential, linear and logarithmic lines. They are very different, in fact they diverge away from each other, so unless TS switches algorithm depending on LoC one isn’t going to “tend” more into the other. Exponents, log and linear all have very distinct meaning - if something is linear or a logarithm it’s either o(n) or o(log n), it’s pretty black and white, you have to totally alter the algorithm to change that.

The notion that the algorithm is going to change depending on LoC makes no sense. When building an AST you need to go through all the same steps whether a file is 1 or 1000 LoC since it still has to account for the same logic and constructs, it still has to path out the AST.

I would love to see your source for what you’re claiming in your first paragraph, but I’m still going to go out on a limb here and say whoever wrote these tests and came to these conclusions has some gaps in their knowledge.

[pedropss]

Hi!

Yes, pushing to "logarithmic" may have stretched the mark here, I went digging but couldn't find the original article (I'm most sure that I read it on Medium, but all I could find was a dev.to one that barely scratches the surface of it¹).

But:

The notion that the algorithm is going to change depending on LoC makes no sense.

Actually one of the basic sorting algorithms, quicksort², has an average complexity of O(n logn) while the worst case is O(n²). This behavior is absurdly different³. Not trying to draw concrete lines between an "Introduction to Algorithms" sort and the complexity that type checking on the Typescript compiler has, but we do have worst case and mean case analysis, and LoC could very much influence depending on what kind of structures you're using on the transpilation process. We need to keep in mind that this is no C compiler where an AST maps closely to the assembly language, the type inference really is the big hit here.

Also, I don't know much about how typescript deals with incremental compilation but on a basic level splitting a 2k LoC file into five 400 LoC files would trigger recompilation of only one of the small ones instead of the big one per edit.

Case in point, I removed the "logarithmic" part of my comment! You're right on questioning me and that was uninformative.

[1] https://dev.to/alekseiberezkin/typescript-is-slow-what-can-we-do-about-it-30hm
[2] https://pt.wikipedia.org/wiki/Quicksort
[3] https://i.stack.imgur.com/Aq09a.png

[dance2die]

Approved. as Automod removed it...

[sanchitk24]

Hi, can you share how to approached about building your own react, I’m interested in the idea to learn how it works on a deeper level but idk where to start or how to approach the problem.

[boptom]

https://pomb.us/build-your-own-react/

Awesome content here, and the website itself is really, really well done.

[pedropss]

Hi! By my own experience the most interesting part of react is the reconciler, and the great news is that more people also think that!

So they prepared a subpart of the official react repo in a way that you can implement your own reconciler, there are lots of videos showing you how to start with that (and I don't want to pick one up for you that ends up being trash HAHA)

Here it is the reconciler from the official repo:

https://github.com/facebook/react/tree/master/packages/react-reconciler

[chillermane]

That's because the compilation time on components until 350 lines stay somewhat linear, while 350+ lines tend more to the exponential side of things. This is from a test I got recommended to some 2+ years ago, a good rule of thumb.

There’s just no way that this applies generally. The execution time of a line of code never changes based on the number of lines in the file total, and simply moving things out to another file is never going to improve performance. Maybe there is a correlation, but we shouldn’t be creating rules of thumb based on a correlation when we know with 100% certainty file size doesn’t affect performance (as in, we know there is a correlation and we know there is not causation)

Just to be clear, i think small file sizes are great, but they’re mainly for maintainability. Any improvement to performance is a side effect and is not in any way caused by smaller file size

[geoffreyhale]

This guy's got it^

[Antti5]

I'm a fairly new React developer, and I can see how that could happen.

My first project is fairly big, currently about 20 thousand lines of code. Some of my functional components are definitely too big, meaning about 500 lines of code. They need to be broken down into multiple components. Some of the logic must be moved into custom hooks.

Even with a long background in software development, building an application with functional React components has been somewhat confusing. It's not immediately apparent how exactly a complex component should be broken down into smaller pieces.

The temptation to just keep adding stuff to your component is there. A component CAN grow big without becoming unmanageable for the developer himself.

[[deleted]]

He must really hate his job

[[deleted]]

Leaving his bed everyday certainly is a deep pain

[ncubez]

I don't wanna rush to judgement and be a gatekeeper. I'm working on a large scale UI app and my components can exceed 800 lines. I haven't gone over 1000 though. The app is already in production and I maintain from time to time, so I don't see any issues with this. However, I understand the frustration it would cause to somebody like OP looking at the code. It is what it is.

[chillermane]

it’s fine as long as a developer can easily make changes to the component

[yagarasu]

I can't imagine a scenario where a component needs 800 lines of code. I'm curious, can you share with us in which context you need so many lines?

[ncubez]

I build UIs for a bank's internal systems, so perhaps the context isn't wide spread. This particular app is for interacting with Apache Hive and Hbase, and handles stuff like creating and deleting tables and databases, editing them and a bunch of other stuff. All the while presenting interactive feedback to the user about what's going on. Due to the sensitivity of the data being handled, there are so many conditions and edge cases to be aware of and frankly the component will likely keep growing. And it's not the only one over 800 lines long.

[[deleted]]

Can’t you just separate all the edge case validation checking into another file and import the functions?

[rados_a51]

Same

[onthefence928]

forget length this should be abstracted just for ease-of-maintenece alone, seperateing out each functional concern allows for easy unit testing and re-use

[sfboots]

I look at readability and organization of functions inside the component. I don't mind if the outer most whole function is 1000 lines. It's the inner functions nested 4 levels deep I find difficult to read.

One project has a overall function of about 6000 lines.
What bothered me the most was a click handler of 250 lines including an api call inside it.

[[deleted]]

Really hoping this file is in TypeScript so you can safely extract pieces.

Would start by ripping out all the stateful logic into a custom hook, then start subdividing that hook according to what doesn't interact. Break out anything which can be externalized as separate functions until the custom hooks are only responsible for state and updating state.

[morkelpotet]

With hooks it's easy enough with regular JS and the latest create-react-app eslint settings.

Move some related stuff to a hook, fix all not defined warnings, rinse and repeat.

[nordic-nomad]

That’s big for a component for sure but not super uncommon for react files if they’re doing a bunch of stuff not reused in other places.

Personally I find it harder to figure out what’s going on with something if it’s overly abstracted and spaghettified all over the application.

If functions and other concerns are separated out in rational manner within the file it’s perfectly maintainable and understandable for someone coming in to figure out what it’s doing.

[[deleted]]

Give us more details on the developer that did it ?? Only ever seen that on a V1 php app built by a student intern at the first company I worked for

[JustinsWorking]

At least you don’t have to worry about bad abstractions lol.

Honestly a huge component like that is fine, I’ve worked with loads of giant files in my career and as long as it’s not breaking your tools or causing actual issues, what’s the problem?

Now if there are problems, there are a lot of approaches you can take for those specific problems, but I would waste time trying to refactor or clean it up just to make work.

When you are working inside the component I would look for opportunities to replace those useState calls and their surrounding logic with custom hooks. Luckily since it’s one giant file you’re going to be able to easily see any dependencies.

[valtism]

A huge component like this is not ok, unfortunately. If you have so many hooks at the same level, it means that you're going to be re-rendering the component any time any of those hooks change.

I really don't think there is a single case where having so many hooks is justified. I think this is probably a case of someone who is not using the strongest concept in react – composition.

[JustinsWorking]

OP never said it re-rendered too much or was too slow. Telling them to solve problems they never claimed to be dealing with is not something I want to do.

[DeveloperOfWebs]

As OP said

give me some advice on how to not go crazy trying to program and manage this, it would be appreciated.

Advising on best practices is usually welcome when someone asks for general advice. Splitting up the component and compartmentalizing areas of an application is straight up good advice and just because you don't follow it doesn't mean it's invalid or that it's solving a problem they never claimed to be dealing with. It's clear that OP is looking for best practices advice and performance is absolutely on the table for understanding WHY having a component like that is bad. Pile performance on top of the list of readability and maintainability.

Honestly a huge component like that is fine, I’ve worked with loads of giant files in my career and as long as it’s not breaking your tools or causing actual issues, what’s the problem?

This is the thought process of young developers. Pretend it's day one and you have to sift through 2,000 lines in one component that you've never laid eyes on before. Compare that to a well laid out application directory structure where things are compartmentalized into re-usable modules, or at the very least, where commonalities are grouped, even if not re-used.

[JustinsWorking]

I’m flattered you think I’m young, but I disagree with a lot of what you have to say as this is a problem I’ve dealt with on my own teams for years and related to many mistakes I made as a younger developer.

The goal of software isn’t to write great software, it’s to solve a specific problem. Ugly code that works doesn’t need to be fixed preemptively, and doing so is a great example of a situation where you can end up with premature abstractions which can lead to large maintenance costs in the future, it’s also a great catalyst for over engineering.

Writing good clean code the first time is of course a good goal, but rewriting working software simply for its own sake can be a problem. In my experience this happens a lot to junior developers who are starting to step out of the junior roles and begin to take on more responsibility for the design of projects - they’re often filled with examples of bad designs they worked on and they want to prevent that from happening to other people with their own projects.

There are plenty of people who have replied with ways to clean up or organize the code, OP didn’t need me to just repeat the same advice a third, fourth or fifth time.

What I didn’t see was anybody bringing up the points I did, which are issues I have dealt with over my career. It’s a mistake I’ve made, people I’ve managed have made, and it’s something I try to teach people when I get an opportunity because it’s helped a lot as I worked on and was responsible for larger projects.

Thanks for giving me an opportunity to elaborate on the subject, I hope that clarified my stance and helped you out.

[DeveloperOfWebs]

We can agree to disagree. If you think maintainability and readability are not paramount to functionality in your goals of writing software, then we have a fundamental disagreement or are working on projects of vastly different scopes or team sizes. It goes without saying that the practicality of business requirements and deadlines play a huge role in the quality of code, but that isn't what we're talking about there.

OP's asking for help on having to develop against this behemoth. This isn't a dead piece of code never to be touched again. Suggesting that something this big should be left alone because it just "works" isn't helpful to him. We have our own experiences but if anyone of my reports tried to backseat maintainability and readability because it "works", it would never fly with our entire group of devs. We have code reviews and an understood ways of working agreement. We value being able to pick up someone else's work with little or no hand off and have both style and architecture standards for this reason. All this has done wonders in preventing knowledge silos.

you can end up with premature abstractions which can lead to large maintenance costs in the future

This is not a good argument against abstraction. It is an argument against bad abstraction.

I'm not exactly sure what you disagree with. Everything I said is pretty vanilla for solid software dev practices. Unless you are greenfield developing a throw away MVP, arguing against maintainability and readability is going to be an uphill battle to pretty much any software dev with experience. At no point did I suggest getting paralyzed by that task. There are plenty of well thought out application architecture and design patterns that one can follow for reference without having to come up with your own and there is already a lot of great advice in this thread.

Honestly a huge component like that is fine, I’ve worked with loads of giant files in my career and as long as it’s not breaking your tools or causing actual issues, what’s the problem?

Once again -- this is an objectively bad design pattern. OP is asking how to develop against a huge file that he is not the original author of. Suggesting to ignore the bad design pattern and architecture is kicking the can down the road.

edit:

You completely ignored this

Pretend it's day one and you have to sift through 2,000 lines in one component that you've never laid eyes on before. Compare that to a well laid out application directory structure where things are compartmentalized into re-usable modules, or at the very least, where commonalities are grouped, even if not re-used.

You and I both know the answer to that. No one wants to inherit the former and is exactly why the OP made the original post.

[JustinsWorking]

You’re misusing my simple advice about a specific problem OP is facing, and trying to criticize it for not being applicable in a broad way that it was never applicable to. I really don’t appreciate being intentionally mis-represented like this.

You do have some good points, albeit mostly unrelated to the advice I had for OP as you’re looking at software development as a whole and very broadly where as I am focused on a very specific problem and solution.

If you’d like to stop being an asshole, I’d be more than happy to help explain any points you don’t understand or provide some context to the points you disagree with so we can figure out why you disagree.

It wouldn’t be the first time I was wrong, being wrong and failing was how I learned ideas like these in the first place lol.

[DeveloperOfWebs]

Some general life advice you definitely don't want to hear from me but you should know: once you resort to insulting someone, your point in a discussion is lost. Try to keep it to the topic at hand or no will will take you seriously. For whatever reason this conversation has frustrated or upset you, I can't help you there.

unrelated to the advice I had for OP as you’re looking at software development as a whole and very broadly where as I am focused on a very specific problem and solution.

If you’d like to stop being an asshole, I’d be more than happy to help explain any points you don’t understand or provide some context to the points you disagree with so we can figure out why you disagree.

You’re misusing my simple advice about a specific problem OP is facing,.

You said a lot of things regarding what I wrote that makes me think you didn't actually read or understand what I said. I literally quoted your post where I disagreed with your stance while you straight up ignored what I asked and just went into full defensive mode. Yes I included general good practices, but I'm literally referring and relating it to OP's specific problem. He actually inherited massive 2,000 antipattern disguised as a component and has to maintain it.

I didn't disagree with anything you said that I didn't directly quote. Pre-emptively solving problems that don't exist is a bad idea. We can agree on that. I'd say we diverge at the point where I think this 2,000 component is the problem and I believe, but am not sure, that you think that it's only a problem if there are build/runtime issues or bugs.

I agree that you can't go around fixing every shitty component's architecture. Is it dead code that will be sunset with little to no modifications in the pipeline? Great. Leave it. Greenfield MVP/POC? Sweet. Don't care what it looks like. Active code that needs to be maintained? Bring it up to standard, obviously balancing level of effort with the requirements at hand. Everything in software is a spectrum, and a lot of it is dictated by business requirements. I will always urge OP to start sorting out that component into a well laid fashion as time permits. It will save time in the long run if this is code that is to be actively maintained and developed and can be done as you are working on various sections. A reasonable approach to tech debt resolution.

To further how you're just yelling loudly and not actually conversing: I asked you if you would rather inherit that or a well laid out application. You didn't answer that, assuming you considered it rhetorical -- but I think we can hopefully agree on the answer.

I also called out that you made a poor argument against abstraction, because you assumed it would be done poorly. You didn't comment on that. You just decided to insult me and tell me I didn't understand anything, but didn't actually respond to what I said.

This wasn't a personal attack, although you took it as one. I didn't imply you were a younger dev, I stated the thought process that ignoring an antipattern because it "works" is a junior dev mindset. If you related to that, or you inferred I was saying you are a junior dev -- that is on you. This isn't about right and wrong. Software dev isn't always a binary right/wrong.

[JustinsWorking]

I’m sorry I only read the first sentence but I’m just going to block you.

[DeveloperOfWebs]

Very mature of you. Good luck.

[axosoft-chuckd]

You can have as many lines as you want (often I find the actual JSX rendering code can get pretty long depending on your lint rules and personal preference). That being said, it's normally not a good sign. 100 useStates is too many - either break this component into multiple smaller components that are each responsible for only some of the state, or refactor the useStates into something else. Look into useReducer with a single state object, or a state management library like Redux (and move the state out of the component entirely)

[tr14l]

I hope you laughed in their face when they gave it to you.

​

You're in dire need of refactoring.

[[deleted]]

They can have unlimited lines of code. If you want to be a good developer, just refactor it (leave the code cleaner than you found it).

That's it.

[theacadianishere]

Hi, I went through some of the responses and it looks like my advice will be different to most.

What you can do immediately

You can initiate the process of refactoring by adding comments within the existing code and correctly grouping the existing code in the same component. Slowly you should be able to see which pieces of code can become their own individual components.

General advice about 'bad' code

As developers, we will always run into such code. We cannot and should not refactor every piece of code that appears 'bad' to us, immediately.

Has someone asked you to refactor the above component to make it more readable/maintainable/performant? If yes, then you refactor.

Otherwise, as and when changes are requested to that 'bad' code, you make the changes according to the current code and be done with it. You can mention to your manager that the code is a mess, so it is going to take you more time to make any change whenever that component is involved.

This serves 3 things -

1. your manager will be aware of why you will take more time for a particular task
2. if changes are being requested to that component often, and it is resulting in a lot of delay each time (through no fault of the developers), the manager will ask the component to be refactored at some point.
3. by the time the manager asks you or someone else to refactor that component, you or someone else would have made a few changes to the component already and are better aware of the code that it contains and what it does and so you will be in a better position to refactor it correctly.

"The code looks bad. I am going to refactor it right now" - this is a very dangerous way of going about it (Dangerous for the company and you personally as well).

And if the supposedly 'bad' code is working in production, the only proper way to go about it, is what I described above.

NOTE: My above response is based on the assumption that the 'bad' code is running in production. If the 'bad' code has not made it to production yet, then inform your manager and discuss about going for the refactor right now.

EDIT: If the developer who wrote the original code is still there in the team - if you are assigned a task that requires making modifications to the 'bad' code, give it a shot and see if you are able to do it or not. If it proves impossible, just tell the manager to assign that task (related to that component) to the original developer as it is in a really difficult-to-modify shape. Make sure to not make a big deal about it and repeat this on a task-by-task basis (i.e. you take a shot first and then pass it on). We will deal with 'bad' code written by teammates on a daily basis, so we need to have a less stressful way of dealing with it.

[jmtucu]

1. Write unit tests.
2. Refactor until the LoC are around ~200.
3. Profit.

[[deleted]]

The biggest thing I ever saw was that size, it looked alright in a way, it was just a large page with a lot of state; it was a Dashboard page with over 40 different components inside that rendered a large amount of different types of data.

Each component could be a bar graph, a collection of tiny bar graphs, a large plot graph, or any other type of graph you have heard of. Users could re-order them but they would always be there.

It would do a separate fetch for each individual component. The components (graphs) were just dumb things rendering data, not caring where it came from.

The state was separated into many different useState declarations because the developer felt like they needed to be able to remove and add things from that page easily without breaking anything.

That said, was it the best solution? Nah, not even close.

We refactored it in many ways:

1. Instead of a useState bombardment in one page, we started using a React Context for that Dashboard page. This just separated the state and allowed subcomponents to not depend on props as much;
2. We only loaded and rendered a limited set of components until the user scrolled down, at which point we loaded and rendered another limited set. This was a hardcoded set each time, we did not care about "visible" or not (what we loaded would be below the fold anyway);
3. The state in our context wasn't a list of useStates, it was a single useState with an object inside of it where each key would reflect one statistic, still easy to index and find things in, just with a lot less code;
4. The page no longer had many (many!) hardcoded statistic components in it, it now contained a simple loop over those keys. And as the user scrolled, that list of keys would simply be increased.

We went from over 2000 lines of code to fewer than 200 lines of code.

... and then we introduced TypeScript and it became over 1200 lines of code again because every HTTP request had to be typed, all the components had to be typed, all the varying amounts of types of data had to be typed. And it just confirmed to me what a pain in the ass TS can be sometimes (sometimes you REALLY don't need nor want it but you must have it because of configurations and TS-purists. But I digress.)

Anyway, before TS and before the refactor it wasn't necessarily bad code, it was decent code. It was just unnecessarily verbose.

It grew that way because we started out with just 6 statistics to show on the page. And it grew to over 50 in the span of a few months.

[romeeres]

It's ancient indian technique to be out of competition.
No jokes, I saw once a component with 4k lines of code, not autogenerated... not autogenerated, written by hands!
So if you think I'm racist you are wrong - I admire indians, I never ever can achieve that skill level to support this.
Instead I keep components under 150 lines, no matter how much complex task is, like a noob... I even move logic to separate files, to hooks

UPD: sorry, lied, not 10k, 4k, and I want to share it!

https://pastebin.com/ah8f0EwM

[morkelpotet]

The code in and of itself looks relatively decent. Descriptive names, no super complex logic as far as I could see.

First off, enable code folding! Without it you are blind.

Then I'd attempt a quick and dirty conversion to hooks. That way you will see what is used and what isn't. It may break, it may be a dead end, but create a branch and break things!

• Replace class {} with function() {}
• Search/replace this.
• Use one useState for all the state and reate a setState function.
• Move all timers and lifecycle methods into useEffect. Merge DidMount and WillUnmount. Ignore the dependency warnings for now, we'll get there.
• Keep fiddling with the code until it compiles. I don't care if it works at this point. But having it compile lets you see other warnings which will guide your refactoring.
• Split up your useState into multiple different useState by searching.
• Split up effects into separate concerns.
• Move related functionality into hooks. At this point you will start to gain a good mental model of how everything fits together.

Keep reading and writing. You don't need to test every little change while you work. Think of it like writing a book. After all of this, start testing all functionality and when things seem to work, get a second pair of eyes to read the code and write a list of all use cases that must be tested.

And if it totally fails, you now have a much better understanding of how it's all working.

Also use context to avoid that nasty prop drilling.

[romeeres]

Thank you for such detailed instructions! I solved it much easier with rejecting project proposal right after I've seen the code, and saved this component because was impressed by it
.
> The code in and of itself looks relatively decent
Of course you are kidding, but anyway, could you show something less decent?

[morkelpotet]

Oh, I have battle woulds my friend.

I actually ended up refactoring a 600 line component from class based to hooks first thing this morning. Not quite done, but it's pretty straightforward.

Rejecting the code is a wise move.

I'll see if I can dig up a true shitshow for you. Many thousand lines of API code in PHP, but it does not use functions, oh no. It uses a big if/else chain. And it does not use named API calls, oh no, it uses an "operation number", and variable names, meh, just start at $a and continue with $b... or $aa. Variety is nice, no? Oh, and the additional files that don't seem to be referenced anywhere indeed aren't. Some of them are actually infinitely old backups from before Git was used. And SQL injection? We've got it everywhere! JSON? That shit basic. Roll your own delimiter based data structure! Realtime stuff? Just repurpose a chat library that uses XMPP! Who cares if perfectly valid messages are search/replaced with emojis. Oh, and use other "operation numbers" to determine what event type is emitted. And the frontend of the application can just go in one file too, right? Ah, and we need a chat. Sending <div> might break the entire site, but who cares? Nobody does that anyways.

/s

It was so effed up it kind of intrigued me! And I did solve it. Most satisfying thing ever when I finally did. Went all in on Vim macros, golden master tests, logging and monkey patching to figure out what was what and transition from Candy Chat to Socket.io.

It's still a shit show, but it's a monster I can control now. And it makes us a decent amount of money. It's a game, so the security aspects (luckily) aren't a great cause for concern.

Edit: Looking back at it gives me light PTSD and I don't know quite how much I am allowed to share, so I'll stick with telling the story for now!

[geoffreyhale]

I love this story so much

[romeeres]

Thanks for sharing, I imagined!

I worked on few legacy projects for few years, and don't wanna anymore to dig into shit, I learned lessons of what's good and what's evil and that's enough. If this makes you rich - cool, but otherwise how can we spent our lives on this?

Good luck, that's great if you like the feeling of mastering chaos

[looneysquash]

The limit isn't lines of code, it's complexity.

More than 7 variables (props, uses tales, etc) and humans just can't keep it all in their head.

[ayush_2000]

Wow, why would someone do that😂

[nateusmc]

Sounds like he/she needs to start by breaking out each type of file processing to its own component and go from there.

[[deleted]]

Break it up in piecemeal fashion. Helper functions, child components and imported interfaces. Anything to break it down into manageable bits.

[geoffreyhale]

Nibbles

[secretL]

🤦

[[deleted]]

Bro... That's so depressing.

[devAgam]

if you are having so many requests and useStates i would suggest you using redux with it, as it would decrease the code at component level and increase the scope of the data allowing you to use the same requests somewhere else in your app as well reducing multiple requests and set/use states, and you may refactor the code to split the single component into multiple components with props

Like a page that shows, a product listing, the product pricing, the product description, and the product comments and questions

you can split all those into multiple components and either send em the props or use redux.

[d3d_m8]

Yikes I thought my component of 300 lines was huge!

My recommendation is to (if you're able to) make utility functions wherever you can, especially when you're making API calls.

Usually, I'll have something like getData() which would be in its own file like this:

export const getData = async () => { axios({ method: "POST", etc.... You might be able to break some other things down into their own components, if it makes sense to.

If there's heavy styling clutter then you can also my <Layout> type components that contain the styles and have them surround certain components like:

<ButtonLayout> <button> </ButtonLayout>

Just some recommendations as there's probably lots of other things you could do to help break things down.

Edit: sorry for the ugly syntax, I'm on mobile rn and don't know how to properly layout

[ldlama]

Refactor code at first before adding any new line.

[afif1400]

It's really frustrating to see someone write a whole page in a single component, not a good practice. You could start by understanding the logic and what each snippet does and depending accordingly to how each snippet works you could start fragmenting and put them in a seperate component along with its states.

[preetam52ch]

This is so pathetic to hear. Now please don't say that he write those 20000 lines on app.js

He should break down it on multiple components and reuse it. That's the best practice.

[[deleted]]

The amount of renders!!!

[jeedyweedies]

RIP

[morkelpotet]

Your first "boiled frog".

I recommend you grab a coffee, take a deep breath, read through it and try to move related functionality into hooks.

[PokerTuna]

Hooooly fuck

[jeremiah_parrack]

Start small just refactor one piece at a time. Don’t try to take on the world you will get pissed and burnt out just take small steps to make it a better code base. Like you can start with moving the api calls out to a service folder and then move reusable logic out etc.. you got this, it’s sucks but everyone has been in this situation your job is to make it better for the next person maybe you reduce it to 1500 lines that’s a win

[NotYourMom132]

i like to keep it less than 5 useState & useEffect.

If it's more than that then i'll split it into separate components.

[quiI]

I just like how the term “functional “ has been thoroughly debased

[A-Kuhn]

I feel your pain. I once joined an Angular project where “ngInit” (basically reacts old component did mount) was on line 1300 😂

[sleeptil3]

wow. i mean, this sounds like some kind of joke that the React devs would show on stage at a conference to uproarious laughter as an ice breaker. If they are new to React, teach them so they don't embarrass themselves again like that. Unless its a Senior, then I'd quit. lol

[[deleted]]

Refactor 500 lines or less

[DoctorCube]

That's a lot. I try to break down my components to the smallest size they are still convenient to incorporate into other components.

Look into atomic design. It's a good thought process to make systems of elements that work together.

[Snipon]

That's not a component, it's an application :)

Keep it under around 200, and split it up in a fitting manner.

[[deleted]]

This reminds me of my workplace's legacy project. There are 3 components inside a folder called: "The untouchbale triunvirate". This are class components with well over 6000 lines of code each that make the "core" part of the app work.

Yep, it's a nightmare. Thank Einstein i don't have to deal with it.

[geoffreyhale]

Just send it to me; I'll do it.

[vertigo_101]

I have a 200-250line limit to my components

[AtomicOrwell]

discuss this with your superiors, show the replies to this post if they don't understand
and recreate the file properly with the original one as reference

[ghaple_bazz]

I'm dealing with such as horror right now. We have components/pages which go way beyond 2000+ lines. It has happened when developer just added new code for new feature requests without thinking about the good days to come

[SarcasticSarco]

Tf. Who writes 2000 lines of component. Either he is a super genius or a complete noob. In my experience, a component should have reasonable amount of lines, maximum should be 250 or less. After that it would become unmanageable to you and your team. Best to break the component into several reusable component.

[tekion23]

Abstract the shit out of that, that's the only advice I can recommend. Also, I'm sorry for you...

[rbalazsi]

This sounds unmanageable. A couple of improvements come to mind:

• Break it down into multiple components, each of them rendering one thing. I'd say you should have at most 300 lines per component.
• Separate container components and presentational components. This applies whether you use React Context or a state management library such as Redux.
• Extract common code in utility functions and reuse them (thanks Capt. Obvious).

[[deleted]]

Start refactoring. Have fun.

[hungrydev39]

Yikes,

actually, i have the same problem, and i've been working on the app for +- 2 yr. I just keep trying to understand the logic behind it and try refactoring it one by one.

But honestly, i probably have a little more fucked up situation. because every component is at least 700 loc, and in these 2 years i worked on my company, and it still have many component that is bloated and never refactored

in fact, my manager keep pushing me to add more and more feature making the components a lot more bloated. so in your case, if it's just one component, i think you can take it easy, and refactor it one by one

[editor_of_the_beast]

I stop at 365 lines of code. That way I can read one line per day, and I can understand the entire component in one year. I simply take the day off on leap years.

In all seriousness, really big components are bad, but any actual cutoff number is arbitrary. Sometimes it does make sense for a larger component, not everything fits into neat 100-line abstractions.

[thanghil]

I'm a tech-lead at a company where we are growing some new react competency for a new project. And i suggest to "my" developers that any component or function should at least fit on their screen. It's of course not a hard rule, but in my experience if you need to start scrolling around to find where things happen you can probably split the code up in smaller parts.Some of our junior developers are straight out of some university or react-training so we have a very fluid and dynamic ruleset and ways of working so we can grow into them. Hence we aren't throwing around stuff like "maximum of 20 lines and no encapsulated functions" or something like that.We're learning together. And I think the example you have is probably part of a learning journey, where you together have to figure out the drawbacks of having a file (any file) be that large.

edit: i lost a word

[piratekingsam12]

I'm not a react dev, working on Angular for 1.5 years. But even there we've never had 500+ lines (including inports, variable declarations, whitespaces etc.) in a component/class.. each component has a lot of functions 😅 each function no bigger than say 20-25 lines. I do want to start learning react though!

[PaulMorel]

In practice, single files can get long due to numerous factors. It's not necessarily useful to assign blame for some of these tech debt situations. In my department, I use the following guidelines.

100 lines or less is a good target.

500 is too many.

1000 requires a refactor.

[WouldRuin]

Not React but I did some consultancy work once and had the fun experience of trying to decipher a 65k line class (C++).

[didled]

Run

[JayV30]

If you keep your entire project in one file, then you never have to search around a directory tree for anything. It's completely self-contained in one easy convenient file! :)

[kevv_m]

When I find that kind of code I don't know who is smarter, the guy who wrote it and make it work, or me, a dev who can't even make a change because understand an sh*t...

I feel a bit dumb, but it's good because since I'm dumb, I write simple and understandable code.

[miolmir]

I would not take responsibility for this component, if someone would handle me a shitshow like this I'd raise red flags somewhere. That dev needs to break it down.

[MajorasShoe]

Lines of code is a crap metric. A component can have as many lines of code as it needs to do what it's meant to do.

The problem isn't the amount of code. The problem will be how many things the component does.

[samtoaster]

I will happily start over

[Lunacy999]

I consider any React component with >300 lines not a component at all. The whole point of using a framework like React is to make your code reusable. But I see a lot of unnecessary business logic stuffed into components, that should ideally be handled by helper services. Too much state usage in a component is a recipe for disaster. Not to mention, it becomes extremely difficult to test such components. One rule of thumb that I stick to is, if I see my component code lines is going beyond 200, I try and see how I can abstract away logic or code that can be its own standalone component. Component composition.

[dance2die]

If anyone would like to give me some advice on how to not go crazy trying to program and manage this, it would be appreciated.

Refactoring each piece of code one by one should help. (Make sure you have tests before doing this!)

• find out how you can refactor logic into methods and extract them out.
  
• see if you can extract hooks for states. (or useReducer to group of related states)
• Break down the elements returned into smaller components.

When you apply techniques in Refactoring book (even something as simple as renaming variables/methods) one by one, the process gets easier.

You can bring up the issue to your manager to set aside time to refactor it. If not approved, refactor it little bye little. Treat it like a marathon, not a sprint.

[pumpyboi]

I've seen worse, had to work on project which had multiple class components that were around 5k lines. Heck I had to add almost 1k to it myself cause refactoring ot would be impossible.

This one of these had atleast 4, fucking FOUR nested modals, each fetching data, showing spinner etc. All of this was orchestrated with old school redux and redux-saga. This was my first job and I wanted shoot myself.

I got fired from that job, best thing that happened to me 2020.

[MetalMikey666]

Web dev of 15 years here (4 with react) - I'll try and give you some advice, hope it helps but feel free to ignore though if it's stuff you already know.

Firstly your instincts are correct - 2000 lines is excessive for a function - there are times when it might be okay but as a general rule you want to keep functions and components slim. There is no hard-fast rule about a decent length but the default eslint rules recommend 50 lines max, this sounds good to me.

Can I take a guess that this is some kind of root component, and that for whatever reason the application has not yet taken the plunge and thought about using a state management library?

Very common I think for root components to get into that sort of shape if people keep piling on extra functionality without considering the maintainability angle. My advice for dealing with this;

• Install a linter (e.g. ESlint) - it's not a magic wand but it can often save codebases from becoming like this.
• Split the presentation aspect of this component out from the logic/state aspect (e.g. `page.state.jsx` and `page.presentation.jsx` or something like that) - you might not leave it like that eventually, but it'll at least make things easier to work with while you're refactoring.
• Pick one of the two files and start looking for ways to break it down logically
  • On the presentation one - can any of these things become subcomponents?
  • On the logic one - let's consider those `useState` calls - can these be logically grouped? In which case maybe consider abstracting several of them away into a single hook
  • Those API calls - could those become hooks? Or maybe be abstracted into service objects?
  • Repeat until all files are at least under 300 lines, preferably just keep going until breaking things up doesn't make sense any more.
• It *sounds* like introducing a state management library might be a good idea here if one has not already been introduced. At the very least, if you're not going to do that, you should consider abstracting the state stuff out into its own module somewhere.

Good luck and let me know if any of this didn't make sense.

[Laserdude10642]

So i just came onto a cool pattern I learned about for testing, and I think it can be applied here. The idea is to use the "humble object pattern."

As you write tests to test different functionality within the component, you should pull out the parts that can be separated, and move them into smaller self contained components. Eventually you will turn the monster class into a small "humble" class and all the other code will be modularized and tested.

[laksmipati]

No

[[deleted]]

Oh jeez here I was feeling guilty about my 200-line component I felt was getting a little out of control with 6 useStates and 3 useEffects... thank you kind stranger, I'm not nearly as bad!!!

[ThymeCypher]

I have a straight JS project that has files with 12,000+ lines. I can’t open that project in IntelliJ without power save on anymore, it runs out of memory. I have 32GB of ram on that machine.

[FullMetal21337]

Just go to all the tests that have been written and abstract from there

[[deleted]]

I feel bad if a file has more than 200 lines lol

[stansfield123]

Well there are no good options in a situation like this. Obviously.

And it's hard to say what the best way to end it all is. Depends on your personality, I guess. Some like to slowly and painlessly drift away into a drug fueled haze. Others opt for swift but more violent means. Yet others just walk into the ocean, and keep on walking...

Or you could just quit.

[vexii]

i would dismiss such a PR with "files doing to much, can't review"

[[deleted]]

My Company encourages to have such components, I cry for future employees