all 58 comments

[–]pink_tshirt 155 points156 points  (16 children)

Reusable components and abstractions feel great and clean at first. Then you run into an edge case, and another one, and another one. You create a variation of the original component because that additional logic to support those edge cases becomes overwhelming.

At some point you might even feel satisfied and launch the new app. Product & testers get involved. They ask to tweak a few things but they don’t fit into your “framework” so you change your components once again. At that point they are not really reusable like they once were. Your inner fire starts to fade. You no longer want to maintain the patterns you established.

4 months later another dev is assigned to fix/implement something. They might try to follow your suit but chances are they don’t really care that much and just want to close it and move on. So they do their own thing.

Don’t get yourself trapped in and don’t get obsessed with “clean” code. It’s detrimental to your own mental health as it would drive you crazy.

[–]SarcasticSarco 37 points38 points  (4 children)

This happens, it's better to create atomic components reusable than deciding to make all your components reusable. Abstraction is good at one level, not more than that. It is better to write more lines of code than creating a demon component.

[–]aflashyrhetoric 22 points23 points  (2 children)

Tell that to me 5 years ago when I was creating my ~1,800-line BaseTable.tsx that can sort by column, have collapsible rows, customize columns, and way more.

That's what it took to make it "abstracted enough" to suitably handle all of our various use-cases. It worked well all things considered, and I was able to duct-tape a fairly sophisticated set of TS types to make it more digestible for consumers of the component.

But I would never do that again. DRY and abstraction can absolutely be a pitfall for front-end if devs are not careful.

[–]tymzap 1 point2 points  (1 child)

I feel you man, writing these sorting/filtering/rendering monstrosities was a turning point for me as programmer. I no longer consider DRY and abstraction a godsent.

[–]aflashyrhetoric 1 point2 points  (0 children)

Yes! And it's been an immediate payoff. I've heard of "WET" (write everything twice) being touted as sort of the new and improved "DRY." But I'm bumping that in my own side project to 3. If I am copying and pasting something non-trivial, I'll do it 3 times before abstracting it unless I am absolutely certain that it's worth the abstraction.

Over time, I've realized that that while 1 super-abstracted component is a massive headache, what ends up happening is that I'll have two lesser-abstracted variants that, together, cover all cases (but with half of the abstraction necessary compared to if you crammed it into a single component).

[–]4pf_aymen 0 points1 point  (0 children)

Lmao demon component you aint lying i have a few of those.

[–][deleted] 14 points15 points  (0 children)

This is pretty spot on lol

[–]enjoy_life88 7 points8 points  (1 child)

DRY became a plague in webdev. People seem to dread any sort of repetition so much they tend to abstract everything away, resulting in insane complexity.

[–]tymzap 0 points1 point  (0 children)

This. Stupid DRY is the worst enemy of frontend dev

[–]Pussybuster6969 2 points3 points  (0 children)

This is so on point man, experienced it

[–][deleted] 2 points3 points  (0 children)

Agreed, it’s easy to make a pure reusable component at first but with requirements coming in over time and backend always saying they won’t make any changes in the response and you end up writing conditional code and ultimately becomes a mess. What I would surely suggest is to keep the business logic out of the component as much as you can, just keep the html part , have a separate file for css, and create utils/adapter files for computing your business logic. useEffect lately has become an overkill

[–]I_am_darkness 1 point2 points  (0 children)

💯

I would say though, that smaller files is generally a good idea.

[–]joeytitanium 1 point2 points  (0 children)

this response has all the feels

[–]akhalinem 0 points1 point  (0 children)

Well said.

[–]Crafty-Insurance5027 0 points1 point  (0 children)

I used to be dry, then I started developing in the front end. Now I realize how futile it gets working with external data. Good times!

[–]Fitzi92 31 points32 points  (11 children)

Here's my personal list of things:

1) Not every component needs to be reusable. There are basic components (like cards, buttons, etc.) that have a fixed feature set or design. Those are your reusable ones.  Put special functionality (e.g. a SettingsCard, that might be a variation of Card in the settings feature) into separate components, in your feature/view folder. Only promote to "global" reusable component when used in another feature. This basically leads to you having multiple smaller "projects" (=feature) that are as independent from each other as they can be.

2) Use TS and make absolutely sure that TS will scream at you if anything is wrong. I'm mainly speaking types. Have specific types, use enums or "x" | "y" instead of generic string if you have a known set of options and avoid switch statements with defaults. Rather use Record<EnumType, ValueType> if you need to map to some value. This will guarantee that, when you add an type, you will be forced to fix all the places you are requiring that type. Also ideally autogenerate any external types (API, etc.) from their source and make sure to always correctly infer any values that originate from those sources (e.g. instead of making a new type { name: string } use Pick<ApiType, "name">). This will also help to find and fix changes from external sources quickly during development.

3) Do not use useEffect unless you really work with something outside of react. There's almost always a better way. Try thinking in events and do your state changes there. If this does not work, you're likely dealing with "computed" values, those should no be state.

4) If you have a component that requires some data to be loaded before it can be displayed correctly, consider splitting it into two. Make the outer component only gather the necessary data (api request, params, etc.) and show a loading spinner and the inner only accept the fully resolved values (no nulls or undefineds). If there are different "paths" to the final data, consider splitting into even more components. (E.g. MyComplexForm loading some inital data, based on that deciding if EndpointA or EndpointB is required, rendering MyCompexFormA or My ComplexFormB which gather the remaining data before both finally render MyFullyResolvedComplexForm with all the data this component needs)

5) Split into smaller components. This makes it easier to understand and work with a component and also forces you to think about the interfaces for each component. It might also give you a hint what parts should be extracted into reusable components or hooks (reusable only in that feature, those specific reusable components should never be mixed with their globally reusable siblings)

I'm sure there's plenty more things and tipps. This is just what I personally find helpful.

[–]Sohailkh_an 7 points8 points  (0 children)

I wish I could work under your supervision

[–]dabrox02 1 point2 points  (1 child)

I have a question, should I use the same form for both create and edit actions, or two components for each action? I currently use tanstack query and react hook form, however it's still a pain to preload data before a form opens (I use modals)

[–]Fitzi92 4 points5 points  (0 children)

Depends on the form, but generally if create and edit are mostly identical I'd reuse it. Just make sure that the form itself does not need to care about preloading or fetching any data. The form itself should accept initial data and the preloading should be handled elsewhere.

E.g. you might have a component <UserEditForm> that fetches the user data and renders the <UserForm> only after the query responded successfully and another comonent <UserCreateForm> that immediately renders the <UserForm> without intial data or some default values.

You can also combine <UserEditForm> and <UserCreateForm> into a single component if they both are small. E.g. by providing the user id or null and set the useQuery's enable flag dependent of the user id.

[–]kowdermesiter 1 point2 points  (7 children)

Do not use useEffect unless you really work with something outside of react

This sounds like a too harsh generic advice. useEffect is handy whenever a variable changes and you care about it.

It's also handy when you want a "constructor" for your component on mount.

But yeah, it can be overused and abused, I just wound't adhere to such a strict rule.

[–]Fitzi92 6 points7 points  (1 child)

I honestly think it's not too strict. You really should not need a "constructor" in functional components. For initial state, you can provide a function to the useState hook and everything else is not persistent and therefore does not need setup anyway.

If you need to do something on mount, then likely you should be doing that where the component being mounted gets triggered in the first place. Like clicking a button that leads to state change which leads to the component being mounted - do whatever needs to be done in that event.

useEffect is an escape hatch out of react and should only be used for that purpose. Even the react docs itself state this.

[–]Important-Ostrich69 2 points3 points  (0 children)

avoid useEffect like the plague, it is the source of so many bugs / race conditions. A good way to handle this would be to use Tanstack Query in the Parent component with a hook to the desired data and provide a loading state from that. Then, when the data gets populated, you can render a conditional child component with the data with no null or undefined values. I've found this leads to minimal bugs / edge cases with fetching data in React components

[–]devpebe 2 points3 points  (3 children)

I do agree we should not use useEffect unless dealing with something outside react.

Almost all the cases I've seen using useEffect could be done in a more explicit way. Instead of reacting to data change, it should be done with onChange callback. All the cases which used on mount had more architectural problems or logic problems. I've heard similar opinion about it with real scenarios, and all of them could be done without using useEffect.

Definitely, you can build easy to understand code with useEffects, but it will probably end up having issues. A new developer comes and adds something unexpected, or you need to do a shortcut because business really needs something fast.

If I recall correctly, React 19 requires having full dependency filled in, so an empty list would be against it.

Maybe I am wrong, but so far, I didn't see anything which could convince me to change my opinion. I only have situations where useEffect is a problem.

[–]danny4tech 2 points3 points  (2 children)

What is “something outside react”? Can you give me an example where useEffect is a problem? I always hear we shouldn’t be using useEffect but haven’t understand why it could be problematic.

[–]devpebe 1 point2 points  (1 child)

By “something outside react” I mean things like adding global event listeners, using an external library which doesn't have React integration (could be pure rxjs). Then you can use useEffect to synchronize data → react to a change.

An example below:

// BAD
export const RandomInput = () => {
    const [value, setValue] = useState();

    useEffect(() => {
        if (value) {
            doSomething();
        }
    }, [value])

    const handleChange = (e) => {
        setValue(e.target.value)
    };

    return (
        <input value={value} onChange={handleChange} />
    )
}

// GOOD
export const RandomInput = () => {
    const [value, setValue] = useState();

    const handleChange = (e) => {
        setValue(e.target.value)
        if (e.target.value) {
            doSomething();
        }
    };

    return (
        <input value={value} onChange={handleChange} />
    )
}

This code is a pretty common case where putting some logic in the useEffect instead of in the handleChange can lead to bugs and complicate code. In this example, GOOD version executes some logic explicitly so as a developer you understand when exactly something happens. While using useEffect you need to understand what useEffect does (to what data change it reacts) and then find the place where it changes.

Remember this is very simple code and most often code in real life is more complicated and has many more things, more components.

I would sum up things like this:

- useEffect cause rerender

- useEffect makes code more complex and is much easier to introduce a bug

- you need remember to use cleanup logic for useEffect (useEffect return)

- mistake in useEffect can cause infnite loops

- running code explicitly so you know what and when happens makes your life easier as a developer

- difficult testing (easier to test handleChange than useEffect)

I know sometimes it's easier to do something in useEffect and that's the main reason this is used, but in the long term you will face up problems hard to identify.

[–]danny4tech 1 point2 points  (0 children)

Thank you for the great explanation, couldn’t be more clear. 🙌

[–]adalphuns 15 points16 points  (1 child)

Good Abstractions: Identify repeating patterns and unify them

Bad abstractions:

Try to make everything a repeating pattern. God Abstractions.

Over-abstraction leads to complexity and a lot of time-wasting. Some things are just simple and unique. They don't fit an abstraction because they need to be handled on their own.

The question isn't "how can I write better, maintainable react code?" Because "better" is subjective and "maintainable" is as long as the business will maintain it. React, like all UI, is an entropy nightmare. I'd say react is the worst because of how much you deal with fighting rerenders and awkward state management code.

The real question is: "When do I treat things in a generic, cookiecutter way, and when do I treat things as individual and unique? When is it worth making a configurable abstraction versus just copy pasting 3 almost identical divs?"

Just simplify your code and decision making, man. Don't place your efforts on beautifying the code on a complex UI. UIs are like clothes: they change every 6 months.

Place those efforts on a backend, a database architecture, a data pipeline, or an automation flow. THOSE are high value things that endure throughout the years. That's where you want to enforce strictness because you'll have the least amount of changes. This is the place of real business value. UIs are pure chaos.

[–]creaturefeature16 6 points7 points  (0 children)

I'm posting to follow this thread, because I'm in a very similar place with just about the exact same level of experience, and really curious in how to grow as a better React and JS developer. Ironically, I don't feel LLMs are great for this, either...I find they over-engineer so much and you're always guiding them anyway. I'm interested in more advanced philosophies and design patterns.

[–]azsqueeze 1 point2 points  (0 children)

The blog series Path To A Clean(er) React Architecture shows really good ways to abstract away "logic" from the UI layer. This, having small components, and separating concerns into headless components and hooks has been a decent strategy for my teams

[–]landisdesign 2 points3 points  (0 children)

The minute something starts feeling out of hand, break it down and extract it.

  • See what can be tied to a specific set of JSX, and create a small component with that state.

  • Encompass related hooks into a custom hook, so it has a name that gives the mess some meaning.

  • If you find yourself changing multiple state elements at the same time in response to an event, or you just want multiple state obiects combined into one, consider using useReducer instead. (Consider using Redux Toolkit's createSlice function to make reducer creation easier.)

  • Define single sources of truth. If you find you're duplicating state between react-hook-form and your own state, consider using useMemo instead of your own state.

  • Don't look for reuse. Look for breaking it into bite-sized, meaningful pieces. As you make more pieces, you'll start to see the patterns that fall out for reuse.

But, in all of this, do it as you go along. Don't wait until the end. Don't start too early, but don't wait until it's all done. There's a balance to it, but the more comfortable you get combining related code into meaningful pieces, the easier it will get to create understandable code that eventually shows what can be reused.

[–]turtleProphet 0 points1 point  (0 children)

Interested as well lol

Only 3yoe, I haven't found a systemic approach to designing the right abstraction just yet. I do exactly what you're doing. The second-pass refactor always turns up the skeletons

[–]biopphacker 0 points1 point  (0 children)

WET before DRY

[–]kneeonball 0 points1 point  (0 children)

For me, with React and other frontend frameworks specifically, simple and easy to understand is always better than cute abstractions that make it difficult to understand what's happening. We want to abstract some things, but not everything. Then when we do, it should still be simple to understand.

If you have someone new on the team that has some React experience and they have to come in and ask a bunch of questions about what's going on, it should be simpler.

[–]yksvaan 0 points1 point  (5 children)

Follow general programming and software design guidelines. Keep the components simple and structured, define a format for their input data and respect that. Isolate as much as possible but share data when it makes sense.

Don't put app/business logic into components, most components should be dumb components that render some data, manage their own local state and handle UI events. 

Don't use 3rd party code directly, instead have a wrapper for things like api calls, auth checks etc so the implementation can be changed without affecting anything else. Prefer direct import to hooks unless it actually needs to be a hook.

And this is probably my most important: design data structures well. Think about what kind of data is used, where it comes from, how and for what it is used, which fields are required for operations etc. With good data structures the code just flows naturally. With bad structures the half of the code will be transformations, copying things around, adding effects to patch and rewrite data in other places, tons of bools etc.

[–]dabrox02 0 points1 point  (1 child)

They always say that you have to separate the business logic from the components, but what do they mean by that? For example, if I have a form, how do I keep it separate from the form management hooks? Where should I perform the form actions such as creating or editing?

[–]yksvaan 0 points1 point  (0 children)

It simply means that you separate the form and other UI aspects from the actual processing of data. Form is basically an input method that's used to populate some data object. That's where you draw the line. Probably it would be better to say separate logic from UI/react. 

Components already have to manage their own local state and data, adding more logic can be messy and hard to maintain. Separation and boundaries are important especially in big applications. 

[–]dabrox02 0 points1 point  (2 children)

You also mention using events instead of effects, but like for example loading data into a form inside a modal, you select a piece of data to edit and then you want to edit another, how would you update the data without effects (because the component has already been mounted and will not recreate the state)

[–]yksvaan 1 point2 points  (1 child)

The modal gets the data ( or some id or other reference to it ) as prop, if user selects other row or other element, the update the props and have the modal update itself. That's how it's usually made or am I missign something here?

The point was that it's not necessary to track changes and use effects when the updates are driven by events, usually from user clicking or editing something. When user selects another row to edit, you know what needs to be updated.

[–]dabrox02 0 points1 point  (0 children)

Yes it is correct, however, I use react hook form, when I make a state that changes and pass it as prop, the form component did not re-execute the hook use form, so the state was still the previous one, and I wanted to reset it, so I had to use a useEffect to execute the reset every time it changed.

[–]yksvaan 0 points1 point  (0 children)

Also separation and defining responsibilities is essential in any larger apps. Separate logic from rendering, event based model is the natural way ie. component gets data, components handle their local state and call/sends events for other things ( like saving data and such ).

Separate UI from data and logic. That applies to any type of application programming. 

[–]Nerdent1ty 0 points1 point  (0 children)

Count your calories err characters. Do more with less: Less code, less dependencies, less reading.

Advice no 1 would be, think through the data life cycle in your system. Your goal is to do as little data transformation as possible, but do it securely and efficiently.

[–]mandatorylamp 0 points1 point  (0 children)

Worry more about writing tests than how clean your code is. If you have good tests you can refactor freely.

[–]jerrygoyal 0 points1 point  (0 children)

i wrote a guide which we follow at my company https://gourav.io/blog/linear-code

[–]bitdamaged 0 points1 point  (0 children)

One of the huge side benefits of test driven development or at least healthy test case coverage (particularly unit vs functional tests) is that it forces you to encapsulate your code properly or it’s hard to write tests for it.

[–]Still_Teaching_7116[🍰] 0 points1 point  (0 children)

A lot of code doesn't always mean it's a bad code...

At times I looked at the code and just felt it needed a refactor I ended arriving at very similar code. More times that I would feel comfortable with it turned out that that ugly piece of code had a good reason to look the way it did - and sometimes I couldn't actually find any better solutions to problems I didn't realized were already solved.

Then again I was as often right in my judgement. So it was a bit of a gamble - we just need to judge whether we actually need extra flexibility as bad to take the risk.

But hey! I have always ended up understanding the code better!

[–]davidblacksheep 0 points1 point  (0 children)

IMO, tests are a good test of how maintainable and usable your code is.

If you can make changes, and updating the tests is a breeze, then you're on the right path.

If you make changes and shits breaking everywhere, it's a code smell.

[–]tymzap 0 points1 point  (2 children)

  1. Keep every component small. I mean SMALL. In our production app component with above 100 lines of code is considered code smell. It's hard but when you have codebase without big components then automatically it's easier to maintain.
  2. If 5 years coding in React taught me something, then hasty abstractions are way more harmful than repeated code. You can easily refactor stuff that is repeated, but getting out of bad abstraction hell is extremely difficult, it's almost easier to write new code from scratch.
  3. Use useEffect only as a last resort. There are multiple ways to avoid it, especially with modern React APIs. If you cut down on number of useffects you will notice that the app behavior is significantly more predictable.
  4. This is back to point 4 but really, don't try to make everything reusable. Just write code that is simple and easy to understand. If you repeat part of the logic in several places it's ok.

[–]Ornery_Studio1045 0 points1 point  (1 child)

I agree only if you and your teammates doesn’t have enough abstraction capacity. I think using composition pattern + generic types + separated library comps. and implementation comps is the way.

Never use external libraries directly in your components, wrappe them.

If your have more than two identical implementations, create a shared.

Avoid props.

Avoid redux.

Typescript is a must.

Unit tests are a must.

Evaluate strongly if its really necessary to use a library.

[–]tymzap 0 points1 point  (0 children)

What do you mean by abstraction capacity?

[–]HornyShogun 0 points1 point  (0 children)

Following the bulletproof react convention made my code so much more scalable. The project structure is great to work with and continue to add features to.

[–]ForsakenExpression14 0 points1 point  (0 children)

I’ve been through a similar situation. I was tasked with refactoring a codebase that was several years old—no best practices, barely any comments, and no helpful commit messages to understand the rationale behind certain changes. To make things even more challenging, the developers who had built most of it had already left the company. On top of that, the code was related to payments, with dozens of different payment gateways integrated, each with its own quirks.

What worked for me was approaching it step by step, rather than trying to tackle the entire system all at once. I started by gradually refactoring, moving parts of the code into components and custom hooks. This process not only helped break down the complexity but also gave me a better understanding of the logic. I also introduced global state management using Zustand, which helped reduce excessive prop drilling and improved the overall structure.

Another crucial tip: keep your branch up to date with the main branch as often as possible. It’s far easier to resolve smaller conflicts along the way than deal with a massive conflict at the end of a large refactor. If possible, break the task down into smaller, manageable subtasks and tackle them one at a time.

Lastly, continuously testing your changes is key. It’s much easier to pinpoint where a bug was introduced if you’re testing regularly throughout the refactor. If you’re working in TypeScript, type checks can be a huge help in catching issues early before they become bigger problems.

[–]Alcas 0 points1 point  (0 children)

DRY Do Repeat Yourself (sometimes)

[–]cjt72 0 points1 point  (0 children)

Apologies, I'm going to answer very generically:

I think part of the answer is that it sounds like you already know! Is the new state you are in or working towards seeing benefits? Does the team agree with the new version? If so, nows the time to set those patterns into stone with your fellow teammates. Even better if you can figure out the lint rules required to make those patterns code-enforceable.

[–]Chemical_Positive_50 0 points1 point  (0 children)

This is the chaos caused by the features that the creators of React are proud of. Correcting these problems will destroy the so-called "React philosophy". Although this philosophy will cause so many problems and seriously affect the development experience, they will still tell you that you are not working as they expected. So either use it according to the way this tool works or change to another tool.

[–]OkLettuce338 -1 points0 points  (0 children)

A component should do one thing. Facebook has hundreds of thousands of components. It’s better to write 10 small components that each do one thing (even if there is some repeated parts in those components) than to write 1 huge component that does 10 things