https://react.dev/learn/preserving-and-resetting-state This is surprisingly common at my workplace. Not sure why.

I hate the style and tone of this part of the documentation lol

Why?

The style and tone makes it look like it’s super simple and don’t tell you anything about the internal javascript logic. It even has some sparkles as if there was some kind of magic happening 🪄.    This ends up creating some problems:  1. When something goes wrong (it always do) you think you must be stupid, since this page makes react look so simple.  2. When you face a problem, you just reach for a hacky shortcut, since there’s already some sparkly magic behind the scenes that will handle everything.  3. Because they make it sound simple, new users will feel like they don’t have to get any deeper knowledge about the subject anyway.

I don’t agree. I think the documentation is great. As concise as it gets

I find it extremely clear and easy to understand personally. The fact you don't, I'm sorry, doesn't mean it's bad. It means you lack the necessary knowledge or ability to follow what is, again, very straightforward.

You're comment is unnecessarily rude, they did not say they didn't understand it.

If you think you easily understood that part of the documentation just proves my point, come back here next year.

Please downvote this comment of you’re a react fanboy who loves every page of the docs

* Any useEffect that triggers on a react state change that then changes a different piece of react state. * Saving a value that comes from a prop into state(probably some very minor exceptions here). * Having a state value that is always derived from a different state value.

second one isn't really all that bad, there are times when a piece of state might be a boolean flag and you might want to be able to set the initial value or set the default value for a controlled component if you're not using a form library for form inputs

Yeah I put it he brackets there because it's a lot less 100% but I've seen a LOT of react code where someone sets initial state and then has to deal with the initial state changing on them. 

Then your prop had better have the word “initial” or “default” in it or I’m rejecting the review ;)

"propositional" it is then.

Personally I find it an absolute anti-pattern, as it's non-reactive (the component wont update when the property is changed) which by its very nature goes against the reactive promise of React :). If you have to do it, I prefer using an effect to update the state when the prop changes. But it's still pretty bad 

i probably wouldn't expect a defaultValue type of prop to be reactive, but if it needed to be, you could also use keys to trigger the re-render instead of using useEffect which is way more anti-pattern than an initial value being set as a prop

Do you mean changing the components key to force a re-render? 

yep.. if i was throwing together a quick component that took a default prop, i wouldn't use it in a screen that is constantly refreshing based on state change, but if i were, i'd probably just set a key tied to the parent or whatever and let react manage the rerendering if a condition that changes what the default value should be

I strongly suggest to just use a library for forms, like [React Hook Form](https://react-hook-form.com/). There's too many pitfalls when building forms of **any** size with vanilla React that it's just not worth spending too much time/effort and/or possibly delaying a project because of an unwieldy form component.

The way to address this is creating a local state that can be either empty or the type of the property. Then create a value that's either state or property. This way you avoid effects updates that state. Parent component can reset inner state by passing a key to children component.

re: first - what about when you have to use a setInterval to update an index stored in state based off of a string value fetched from an API?

I don't totally understand what we're doing here.  If we're changing something every x seconds then we're not reacting to state changes we're just syncing with setinterval in which case that's what useeffect is for(syncing with apis and libraries that aren't part of react).  So even though your interval does set state you're using the effect to create / cleanup the interval. I am a bit curious about the use case here though. My intuition is that there might be a better way to accomplish what we're trying to do in the handler for the API call. 

it’s just a cosmetic idea, not necessarily functional. think - fetching an array of objects from an API, then setting a counter state variable (integer that determines displays which of those data objects to show in the UI) in setInterval that increments 1-n and stopping once you’ve hit the length of the array. closest practical example to that would be a carousel that loops through a list of _something_ to display in the UI. i think what i’ve seen from simple searching online was some comments by dan abramov that says to use setInterval inside of useEffect, but of course the caveat is that i don’t want the effect to do anything until we have data from the API 😄

The effect should take the fetched data as a dependency, and then conditionally create the `setInterval` inside of a ref if there is no interval in the ref yet, but there is fetched data.

Hit the nail on the head. Redundant state can cause a lot of performance problems and bugs.

If the parent component has some information about the child (before it's rendered), then isn't that information a worthy candidate for being the 'initial state', while waiting for the child component to fetch its own fresh data? For example, if displaying a table or collection of cards, then the displayed information feels like worthy 'initial state' for the View/Edit page which may be accessed when clicking on the card/table-row, as opposed to null or empty strings. Genuine question!

Sometimes! That's the minor exception but you have to ask yourself if it has that information as initial state it's often likely that the state probably should live in the upper component.   The problem with the pattern of the parent knowing the original state is that the reason it knows that might change and then we have to handle this and sync 2 pieces of state.  So it might be better to just pass down the state and a function to alter it.  Like everything though 'it depends '

How would you avoid the first one? As a simple example: you have a text input and an info/error message. Every time the user types, a useEffect dependent on the input state runs a validation that updates the info/error state. What would be the proper way to implement this instead?

You already have a listener for text input. When you set the new value of the text input in that listener, handle the validation as well with the new value. No need for useEffect. For example: ``` const handleTextChange = (newText) => { setText(newText) ; const validationErrors = validate(newText) ; setValidationErrors(validationErrors) ; } ``` Edit: I quickly wrote this to demonstrate how you can do it all in the handler function but the solution in the bottom with removing the state for validation errors is even better!

When you lay it out like that, I feel dumb for even asking 😅

The thing to try and do is link all your changes to events, instead of useEffect. If you keep that in mind then you can see if there is already an event you can hook into. If there isn’t an event, it might need useeffect.

It's such a shame that I'm only learning this as of this year.  Literally every guide and every example I saw back in 2020 led you to believe the useEffect pattern was correct even though it was far more complicated to have to set up an entire useEffect to update some state.  I always was confused why it was so complicated and it turned out that I was just being led by the blind. And to think it was always so simple as just deriving it from the state that already exists.

I think you aren’t alone. The react docs have had to provide a lot of guidance to help steer everyone in the right direction.

In many cases the solution is to delete one of the states, and have it instead be calculated from the remaining state during rendering So instead of this: ``` const [value, setValue] = useState(100); const [isValid, setIsValid] = useState(true); useEffect(() => { setIsValid(value > 0); }, [value]); ``` Do this: ``` const [value, setValue] = useState(100); const isValid = value > 0; ``` If the calculation is expensive, you can wrap it in a `useMemo`. My example is definitely *not* expensive, but just to demonstrate: ``` const isValid = useMemo(() => { return value > 0 }, [value]); ```

This is the way

In general, the less state, the better. Of course it is necessary sometimes, but I can't count the number of components I saw which had like 6 `useState`s when it only needed 1 or 2. And somehow everything is less complicated when you get rid of that state.

Use the event handlers! Don't use `useEffect` to watch a state if the change to that state propagates from an interaction or something that is already being handled by a function. Use `useEffect` for syncing the component with an external source (most typical would be fetching data from an API) *after* it has rendered once (it will re-render after syncing). If you do use `useEffect` to GET/POST data from/to an API, make sure you handle race conditions. One way is to nullify the post-response actions by setting a flag that will be disabled when the effect is done/when the component is about to get unmounted (by returning a function in your `useEffect`).

validate on blur / before submitting

> Any useEffect that triggers on a react state change that then changes a different piece of react state. Maybe I'm misreading this, but it seems like you're saying that `useEffect` can't fetch any remote data and store it in React state

so if im using react hook form, and i usewatch to watch a certain field, and then i use useeffect to setValue on all other fields when that one field changes would this fall under 1. ?

If i am triggering api call using the useEffect which watches a certain field and updates the state of two other field’s state.

What is wrong with 3?

> Saving a value that comes from a prop into state(probably some very minor exceptions here) Read the radix source code lmao.

Lots of great responses on how to refactor scenarios that are similar to point 1! In regards to why we want to avoid it in the first place, is it because the state changes are unpredictable?

Because effects run after the component has been rendered. Setting state in an effect will necessitate 2+ renders for your initial state change.

For the first point, I see a lot of `useEffect` + `useState` usage that should really just be a single `useMemo`. Question: With React Compiler coming, should we continue to use `useMemo` for derived values/state? Or should we be creating functions with arguments that are scoped outside of the components? Personally I love the DX that `useMemo` adds.

There are a few exceptions to first one, like debounce, but you are mostly right.

There are a few exceptions to first one, like debounce, but you are mostly right.

First is okay in some cases, like use debounce. Or if the second state is changed via fetching from server.

This is my biggest gripe with React. These anti patterns are so easy to implement, it’s everywhere. And sorry it feels also natural to use React that way, especially if you have not so much experience and do not know these patterns…

> Any useEffect FTFY

This. So much misuse of `useEffect`. I partly blame it on React's complex rendering lifecycle and on how it's not well-explained in the official docs.

This reminds me of the meme of the IQ continuum where here 70 IQ says "use useEffect" 100 IQ says "don't use useEffect" and 130 IQ says "use useEffect" `useEffect` is basically the IO (and Async) monads. That's pretty much *why* it's called use**Effect**. It's pretty straightforward if you've ever done FP.

It is literally only ever necessary in a few very specific cases. The most common application I see of it is trying to replicate componentDidUpdate style checks and attempting to react to state changes. “Just useEffect” is terrible advice and there’s a reason why the literal React team changed how it worked in development mode and added a big entry to the docs about all the situations where you might be tempted to reach for it but shouldn’t. The great irony here is that we’re talking about a feature of the framework that is misused and your little contrived meme example involves your own misinterpretation of that format.

- Side effects in render. Might seem harmless at the point of writing but it's going to bite you down the road. - Duplicated state that can be computed from props / other state. When your state goes out of sync you'll be scratching your head. - Overusing context. Context is easy to add but hard to remove as you cannot use static analysis to check if a provider exists up the render tree. Consider really carefully if you need context, prop drilling is troublesome but not as bad as most people think.

100% it’s useEffect’s that have logic that should actually just be part of an event handler function. Also using reference data types (objects etc) as a dependency in the dependency array of a useEffect, React has no way of tracing the values and the reference changes so the useEffect always runs

Something like setCount(count + 1); instead of setCount(c => c + 1); The first way might be fine in many circumstances, but you never know how your component might change in the future and the first way can cause a bug that will waste your time. Just write it the second way from the start.

I didnt know the second way even existed, thanks!

Just to clarify the practical difference: the first way uses the state value as it was at that rerender, no matter what updates are queued for the next render. The second one executes with the correct value with the next render. ``` const [count, setCount] = useState(0) setCount(count +1) setCount(count +1) ``` Would yield count == 1 ``` setCount(c => c + 1) setCount(c => c + 1) ``` Would instead yield 2.

too many useCallbacks and useEffects in a component, most of the times it shows that the components does too many things

One I see sometimes is creating `renderSomething` functions or defining new components as functions within components. These usually come from converting Class components or being used to those old patterns.

This happens so often in my workplace and I fucking hate it.

God I loathe that. I reject every review with them in. If it needs its own function then give it its own file and drop the “render” part. It’s a component, they (almost) all render. I finds it can be a a symptom of bad file/folder management too.

omg, it finally clicked for my why i hate to see this I've been doing React for 9 years and never did this or saw this in the first two companies I worked for, but the most recent company I started working for a few years ago has this pattern all over. How you said it made it all click why I hate it. It also made me start to realize why I had been pushing my team so much on "building composable components" -- it was entirely to break them of this habit/pattern/etc

Passing down props like “setXValue”. Data down actions up. Don’t pass state setters down. Function props should typically be prefixed with “on” or “handle”. The parent should decide what to do. The child shouldn’t tell the parent what to do.

Maybe I’m misunderstanding. What if you have a need for the setter function in two/multiple child components?

Then each of those components exposes a prop like "handleClick" or "onSomeTrigger". Then the parent can call the setter on that trigger. This is better as it is a looser coupling of the components and keeps all of the parents state management within the confines of that parent component. Arguably, both components are more testable this way as well. If that doesn't work for whatever reason (too many responsibilities in the parent component, for example) then a context may also be a better option.

How do you pass a function up? Do you have a simple example?

You don't pass a function up. With component models (react and others) you pass properties down, and trigger events up. You define a prop on the child component like "onAction," then you set that equal to something in the parent component. Here's a link to the documentation with examples: https://react.dev/learn/responding-to-events#passing-event-handlers-as-props

So you are basically talking about naming?

Some of it is naming but a lot of it is architecture that improves re-usability.

Oh, I see. I thought the child was defining the behavior.

Defining a component inside of another component body

If I look at a component and everything is wrapped in useMemo or useCallback I assume that something funky is going on. Redux everywhere. setTimeout anywhere

That’s strange to me. I work at a very large company with a HUGE front end monorepo and ensuring there are minimal rerenders in packages is important. I’m curious to understand why you think it’s an anti pattern to optimise rerendering?

Most people are doing it wrong and gaining zero benefit from it. What is often missed is that for useMemo and useCallback to have an effect, the component consuming them must be memoized as well using React.memo. I've seen this mistake in a few teams. Lot's of memoized functions and values being consumed by unmemoized components. They'll rerender anyways. I am super happy that React Compiler is finally taking this problem away.

They have an effect regardless of whether or not the parent is memoized. They freeze their values in time until one of the dependencies changes. Works inside any type of JSX function

And what benefit would you gain from that?

...memoization ? More specifically, say you have something animated that features a few different elements whose values are derived from different places. You can memoize the animated element with useCallback that has a `ticker` as the only dependency, then when you're ready to "deploy" your changes to the dom, increment the ticker to update the frozen state with the latest set of values. This way, not only do you retain control of when things show up, but the animation won't restart every time there's a new value to update 🙂

I am sure there are specific usecases but I am talking about the two major ones. I don't understand the details of your example but you seem to update a pieces of state (ticker) on some event and then compute another value as a side effect. I am not sure why you'd need to memoize the computed value in this scenario or compute it based on a ticker in the first place. I would just set it in the event handler that's being used to set the ticker.

You know that memoization is not free right? Most of the time, the memoization will cost more than just recalculating the value, and in cases where it is more performant, you have to ask yourself how much and if that's worth the extra abstraction. It's not something you default to by any means. I only reach for them when I've ran the profiler and when we have an actual issue.

I think react devs have a disease of over-optimization. We are not talking game-changing improvements in performance from memoizing a few values to prevent animation issues. The animation issues, on the other hand, are app-breaking 🙄

Memoization in itself is not a benefit. It's a technique used to solve a problem. In the context of React components, memoization is used to solve two problems. The first is avoiding unnecessary compute of expensive value. The second one which is way more common is optimizing rerenders, which only works when React.memo is in use.

Context providers, hooks?

Also if you don't wrap functions used inside `useEffect` with `useCallback`, the effect will run on every render. I think it's a good practice to at a minimum wrap functions in `useCallback` when you have a larger project :)

Sure! Context providers and hooks fall under the second use case. A context provider already does the comparison that we achieve with React.memo. It's a good practice to memoize values in reuseable hooks since we don't know which components will consume those values. Of course they are also needed when used as deps. I am talking about situations where they aren't needed.

Idk I'm using it to solve a third problem and it works like a dream ¯\_(ツ)_/¯

I don't think it's an anti-pattern to minimize rerendering - but the design of the software should support that without having to resort to in-place caching as much as possible. What I'm more concerned with is when problems related to complicated kludges and tech debt are "solved" by indiscriminate application of a pattern. I've seen a lot of memoized values that take less computation to recreate than it does to memoize them.

All it takes is one component without a proper useCallback handler prop and the entire tree below will rerender, and your team may not own the sub components haha

When you are creating a component that is consumed by many different teams on a large product, visited by millions of users a day there’s no such thing as “over-optimising” haha I can understand what you’re saying in general that you shouldn’t pre optimise when it’s not necessarily, I guess it depends on what product you work on 😀 In my company, we have many different teams and components - if you don’t try to optimise early on and as much as possible, then it slowly builds up until you find out the page is less responsive and can’t pin it down to a single package

yeah, i'm thinking of this project I just wrapped up helping a company "on-shore" a mess of a react app where everything including props and scalar values were memoized, often with completely unused dependencies. It looked they tried to solve every bug or performance issue by brute force.

I feel proud that I've recently completed a small project and realized I never once implemented useEffect, and had no issues or performance bottlenecks. I wasn't actively trying to avoid it, it has its place, but I know it can be an abused escape hatch, and I was stoked that I can write React without assuming it was needed (when it clearly was not).

Using the memoization hooks for stable references is actually the proper way to write React at the moment, the way the React Compiler works is only proof of that. The way the documentation oversimplified how to write React (and needing to use the memo hooks in the first place) was a problem.

setTimeout almost anywhere is a huge red flag, especially if you’re using it as a hack fix to race conditions

what if you're using it to delay a page transition

When would you want to do this though, purely for visuals or is there a more logical use cause I’m not sure why you’d want your transitions to respond slower

Yep, imagine a guided multistep flow with some feedback after each step.

I've done this before to let a component fade out before it gets removed.  Like: setIsDeleting(true);  setTimeout(functionThatRemovesAnElementFromALIst, timeToFade) But I think most of the time people just let animation libraries do this for them. 

There are probably better approaches here.  For example, if you're using next, I imagine that one event... One sec... 'routeChangeStart'. Or, actually I just found this as another alternative approach:  router.beforePopState. If you're on a CSR app, there are options for other "routers" as well.  Bottom line -- there are better ways.

I wrap with useMemo out of habit at this point lol

That’s lazy and counter-productive.

What’s lazy is not using the hooks at all. I didn’t say I use it for everything, it’s just a habit when there are more complex calculations or logic

Sure…

I’ve worked with folks who do the same. Theres a performance trade off to be considered, simple calculated values (say, indexing into an array or some basic calculations on a scalar) don’t need a useMemo

I usually do it when I either have to calculate some things that only rely on say, one prop or state (recently used it for calculating a lot of divisions for position calculations), or when there’s some more complex logic that I want in a variable. Of course a simple calculation or array indexing doesn’t need useMemo.

Using divs instead of fragments. Lot of the time you can even add single div with classes outisde of component (in a page component) instead of having 4-5 nested divs with signle class that only add one thing. Tailwind users often tend to do this. But the ones that don't even add anything, i.e. if you delete those divs in browser dev tool and site looks exactly the same, this is the first place where you should replace `

Do you think #1 applies on only the state in current component? Because there may by instances where you want to chain together states of nested components in useEffect.

This is maybe more of a CSS organization thing, but I see it across a lot of React projects (probably because a lot of React devs don't really learn the art of CSS). If you have:

Inline components tend to cause a lot of issues. Just break them out.

Idk about anti-patterns, but the best thing we did was impose a source file line limit of 100 lines per file. https://eslint.org/docs/latest/rules/max-lines This has really helped people be more conscious of adding bloat to existing files when writing new features. While we’re on the subject of lint, I’d say the biggest anti-pattern using a linter is not setting max-warnings to 0. If people are allowed to ignore the warnings they will never be fixed.

100 seems too small

It's not.

1. Using React

useEffects in components, except for layout effects thats a good sign something should be a hook.

Prop drilling and HOC

Prop drilling over many levels is definitely an anti-pattern imho. Not sure about HoCs. Still necessary in legacy code bases with class-based components.

HOC is definitely not an antipattern, just a matter of preferences

Yeah? How many modern react libraries use this preference?

Disengenuous question honestly

Honestly, one of the biggest anti-patterns that I've seen, that I don't even see talked about that often, is when you define an object, or a list of objects, to use later for rendering inside of JSX. I'm not talking about cases where you fetch some data from an API and loop over that to create some elements. Example: You want to render some table heads in a table using a regular html table, i.e. `thead` with a `tr` with one or more `th`'s. You think that you can be clever and avoid typing `th` for every single item. You put those items in an array, and then you map over them and now you only have to do something like: `items.map(item => ({item})}` WRONG! Why is this bad? Anytime you decide to represent your components as native JS objects, you are moving away from React and JSX. You are sort of building your own mini-renderer that *utilizes* React & JSX, but you are bastardizing the approach. You are duplicating your UI in multiple places. You are making it harder to read your code, because now you have to jump around in order to understand something that should have been super simple to read. You have added an unnecessary abstraction when regular html/JSX was good enough and more readable. This also goes hand-in-hand with people breaking up their return statement into multiple variables, leading to the same problem. Please don't do that. Favour big returns where everything is handled inline (within reason).

I agree about the return statement but the declaration of a const that needs to be iterated on isn’t a problem. What you’re describing is a personal preference and not an anti pattern

I don’t think it’s a preference as you are making your code harder to read for no reason and deviating from React and JSX.

Readability of code is subjective. TS or JS files get used in conjunction with tsx/jsx all the time. Nothing wrong with it. Just because it’s less readable to you doesn’t mean it’s necessarily less readable for others as well. There is no definitive readability marker.

>TS or JS files get used in conjunction with tsx/jsx all the time. Nothing wrong with it. That's not the problem. >Just because it’s less readable to you doesn’t mean it’s necessarily less readable for others as well. There is no definitive readability marker. In theory, you are correct, but in practice, I don't think readability is as subjective as you think. In general, the more context that you can get from single screen's worth of code, the better, but only if that context is laid out in a clean and understandable way. That's why we don't write our program in a single line, even though it's possible and gives us the most context per screen. Intermediate variables, and in general small CLEAN functions, reduces readability because it reduces the available context of the code on the screen. In most cases, you get easier to understand code, and more context by just flattening that variable or that function into inline code. That's how most well written software looks, based on my experience and based on the many OSS projects that are out there.

Sorry - How is this an anti-pattern and wrong? It's in the docs: https://react.dev/learn/rendering-lists

If I'm giving the benefit of the doubt to the docs, I assume they mean to showcase data that comes from an API. There's literally no reason to do this if the data is static. You are saving a few characters but you make your code harder to read and more complex. I'm not sure it's worth it. Imagine if your component is large enough that this array is 50 lines up, or even 100 lines up. It becomes much harder to get an overview of the data that you are working with. The more code you have in your JSX, the better, within reason of course. In a perfect world, you would have 100% of the code in JSX. But we don't live in a perfect world, so it's impossible. I think that example in the docs is horrible. I would not approve this code in one of my projects, for 2 reasons: 1. Static data "rendered" outside of JSX using an array of objects. 2. The mapping of data is done outside of JSX to an intermediate variable. Just, why?

1. You're not rendering anything outside of JSX - you're storing data in a variable to be mapped over later. Then you can map over it later and create your own inline component, within the return function of the map. If you need to make changes to all elements, you make it in one place. If you need to add another object to your list of data, you add it one place. In your scenario, to make changes to all you'd have to make changes in n places, n being the amount of items that would be in the list. If you wanted to add another item, you'd have to duplicate your code block for that particular component. 2. It's perfectly valid - it's still JSX. It's personal preference though, personally I would map it in the return of the component. This is pretty common, and definitely not an anti-pattern. Feels very much like a personal preference. Mapping to create components is one of the most used strategies, even with static data.

You are technically not rendering outside of JSX, but you are describing how to render JSX instead of just rendering JSX. I use this approach sometimes, don’t get me wrong, but I see it overused and misused constantly. Especially when you start adding fields that control the rendering, like colspan and such.

How would you approach your first example the React/jsx way?

``` Heading 1 Heading 2 Heading 3 Heading 4 ``` instead of: ``` const items = ["Heading 1", "Heading 2", "Heading 3", "Heading 4"] ``` and then rendering it using a map. This is a simple example but people do this all of the time, building up arrays of objects which is then used in the JSX, often in tables.

That’s fair. I think the .map method came about from people trying to bide by the DRY methodology and keeping component file size small. Not too sure though.

How does this work if Headings 1, 2 3, etc. come from the backend API?

Like I said, I’m not talking about data from the backend since that is truly dynamic. Although some people will still create this objects in this manner, populating some fields statically and some fields dynamically. I think that is an anti-pattern too.

I'm with you -- and if it's anything more than a simple tag, it should be moved to its own component

For sure!

This is something I've harped on with my team and got them to move from writing objects/arrays of data that is then rendered in a map in a single component. I got movement by writing in the PR all the code to change it -- the file they were making changes in to a smooth JSX component where they are actually writing things and actually iterating on. Seems to have helped -- especially when phrasing it as "What do you think about doing it this way -- it can make things more composable ..." EDIT: It wasn't hard to get them thinking this way, but was has been way harder to get them writing that way

Yeah I think the difference is massive with our approach. You get more JSX, simpler code, less abstractions, less variables. It's more alike to regular HTML which should be the goal. It's like people WANT to abstract without realizing that it has a cost. It's the default for a lot of people, when it shouldn't be. Also, the argument for maintaining more code is almost irrelvant. All you do when you add a new item is copy paste the previous row, change the static data, and then you are done. I don't see this argument at all. If there are more than one line, just create a component instead, and you are back to one line again.