Juraj Pavlović for Bornfight

Posted on Nov 15, 2021 • Updated on Nov 22, 2021 • Originally published at bornfight.com

Bad ReactJs practices to avoid

#react #javascript #webdev #engineeringmonday

There are plenty of articles and blogs that contain useful information on how to do things the right way. Best practices, good design patterns, clean code style, proper state usage, etc...
Therefore I've decided to take things the opposite way and look for how not to do things!
This article will state the bad practices and combine them into an enjoyable read.

Using state for too much stuff

While a ReactJS state created with useState or useReducer is useful, not everything should be placed within it. A lot of new developers struggle with this very concept. They do not know when to put something in the state and when not to.

An example would be storing data in the state which should have been derived from the state. Let's say you have a state that represents a filled shopping cart on a webshop. The bad practice would be setting the total price inside the state too. One can simply compute the value from the state already.

Simple computational tasks or variables exist for that particular reason. The usual idea is to store as little data as possible in your state. Before you place data in the state, ask yourself if you can get the needed data from the other stored variables or state.

Using Redux when you don't need it

I just had to put this eternal React developer's debate in here. Developers ask and say stuff like: "Should I use Redux or should I use Context?", "Just use Context instead of Redux" or "Is Context a good replacement for Redux?"
There are many tools and mechanisms that partially do what Redux does. This in short explains the questions and statements mentioned above.
Let's try to settle this debate once and for all.

Redux & Context
Many developers tend to think that Context by itself is a state management system. It is not! Context is a dependency injection mechanism.
Inside it you can put anything your heart desires, it can become a state management system if you implement it that way. One has to use useState and/or useReducer hook to manage the state inside it. That way, you are deciding where the state lives and you are handling how to update it and where exactly you wish to use it.
Context was made exactly to avoid passing data through many layers of components. If you only need to tackle this issue, then just use Context.

Redux & caching
Most applications need a cache of some sort for server state these days.
If we stick to the REST API's there are a couple of libraries that do the caching for you. For example React Query or Vercel's swr both do a good job for REST API.
If we use a more modern approach with GraphQL, caching is really easy with Apollo Client.
If caching is the only necessity in your app, you do not need Redux in your app at that point.

What is Redux used for then?
Redux is a generic state management tool. It has a lot of use-cases simultaneously. The most noticeable ones are: Caching state, UI state, complex data management on client, middlewares, etc.
In the end, it all depends on what specific problem is the app you are building trying to solve. Usually, you will only need the partial Redux features (global state management, caching).

Declaring components inside of components

This is severely bad because of a multitude of reasons:

The code becomes very coupled. The inner components become dependant on the scope of the parent's component.
Inner components are almost non-reusable. You cannot export the inner components, you can only pass them as props further down the scope, which is not ideal.
Performance. On each parent's component's render, the declaration function for the inner component will be re-created. To explain this further, inner component's lifecycle methods will be called each render cycle. Along with the performance issues, the previous state will be lost as well.

Keep the components in their respective files to avoid this issue.

Using props in Initial State (in some cases)

Keep in mind that using the initial state for generic components such as counter component from the official React docs is perfectly fine. In a more detailed manner, this means setting props to state in order to initialize the state of a component with a non-reactive prop.

Outside the provided example, the initial react state should not be set to a value of a reactive prop. Why? Well because that state will not get changed unless you call the state setter, a setState function. If the props from the upper level get changed, the component will get the changed props, however, the state will stay the same as the initial prop value.
This issue destroys the single source of truth concept used in components. It is a bad practice and it should be avoided.

Using index as a Key

You render multiple items in React with array.map method. Keys must be unique so that React can handle proper tracking of that element or component. If you were to use the index as a key, that key can be a duplicate in some cases, which should be avoided.
Imagine having an array of items that you are going to render via .map and use the index as keys. Furthermore, imagine adding to the middle or removing an item from the middle of the array. Key will end up being the same as before, and React will assume it is the same identical element/component as before.
This could lead to undesired effects and should be avoided.

Using the spread operator frequently

Use-cases of spread operator are great. It helps us reduce the code and manage it in a more clear way if used properly. Spread operators are nice when declaring reusable components or creating new data objects that reuse data and even when passing arguments into a function.
However, a lot of times, developers make a mistake of using a spread operator on props and setting wrong or undesired props on a component. This can result in the following error showing up in the console:

Not using useEffect, useMemo & useCallback dependencies

The stated React hooks introduce a concept of dependencies. This is just an array of items which, when changed, will cause the hook to update. Managing the dependencies can be a bit tricky if you haven't done such a thing a couple of times. Dependencies array should consist of items that reflect the hooks and should not be crowded with a great number of those items.
ESLint static analysis has a rule that can help us use dependencies in those hooks.

The dependencies array can only be empty if you intend to use useEffect once when the component mounts.

Doing premature optimizations

Doing optimizations is usually a good thing, but it should not be done for every little tiny thing. To see the benefits from memoization, it is required to use hooks like useMemo or useCallback and even PureComponents. Developers need to be very focused and implement memoization with proper care because otherwise, it can break memoization one by one.
The following image says a thousand words:

Badly declaring TypeScript types

Most of us have grown to love TypeScript and cannot develop in JS without it anymore. Furthermore, most of us know about keywords known as any, never and unknown.
Unknown represents a set of all possible values, any value can be assigned to a variable of such type. It is a type-safe counterpart of any
Never represents an empty set, which means no value can be assigned to a such typed variable.
Those should be avoided most of the time. This can't be stressed enough. Developers tend to be frustrated at TypeScript and then just write one of these keywords to get it off their backs. This is a bad practice and should be avoided.

There is a place for using these keywords, but it should be done scarcely:

Use never in positions where there will not or should not be a value.
Use unknown where there will be a value, but it might have any type.
Use any if you really need an unsafe escape hatch.

Conclusion

There are many bad patterns we came across today and also how to avoid them and use proper patterns instead. If you learn to avoid these bad patterns, your life as a coder will be much easier and you will avoid many bugs and potential refactors.

Thank you so much for reading!

Resources:
https://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html
https://isamatov.com/react-derived-state/
https://changelog.com/posts/when-and-when-not-to-reach-for-redux
https://blog.logrocket.com/when-to-use-never-and-unknown-in-typescript-5e4d6c5799ad/
https://levelup.gitconnected.com/react-best-practices-avoiding-bad-practices-fefe6062787d

Top comments (25)

Juraj Pavlović • Nov 15 '21

The issue with passing the prop to the initial state is that one may think the state will change if the prop does. This could make young developers confused. With that, your latter example makes much more sense. Keeping the state in a container and passing the state handlers down to the presentation component.

Thank you for your contribution.

James Thomson • Nov 16 '21

The issue with passing the prop to the initial state is that one may think the state will change if the prop does. This could make young developers confused.

I don't think that's a good enough reason. If you think this way, then you don't understand how React's lifecycle works and that's a far bigger problem - avoiding a perfectly acceptable practice (which is discussed within React's official docs) only accentuates that problem.

Juraj Pavlović • Nov 16 '21

It doesn't make a lot of sense in a way of how a React app should have its state structured. If a prop is higher up in the component tree, it shouldn’t be used as a part of the separate state within a component. What is the benefit if you do such a thing? It makes sense to use the prop to determine the state implicitly, but not to directly copy the prop in the state. This is clearly a code smell and it is discouraged by many.
Another thing, regarding the React docs, I think you misread them. The link you provided only offers an example with useState and some initialState inside, it does not state or show using props as initialState. This is what actual docs say about it: “There should be a single “source of truth” for any data that changes in a React application.” and “If something can be derived from either props or state, it probably shouldn’t be in the state.” In another words using props to generate state often leads to duplication of “source of truth”, i.e. where the real data is. Meaning this is a bad practice.

Other than that, there are plenty other articles, blogs and the React docs covering this topic which agree with the stated:
reactjs.org/docs/lifting-state-up....
stackoverflow.com/questions/400634...
medium.com/@justintulk/react-anti-...
vhudyma-blog.eu/react-antipatterns...

James Thomson • Nov 16 '21

If a prop is higher up in the component tree, it shouldn’t be used as a part of the separate state within a component.

Well of course, but you wouldn't use initialState in that use case.

It makes sense to use the prop to determine the state implicitly, but not to directly copy the prop in the state. This is clearly a code smell an it is discouraged by many.

Yes, again, if you're doing this then you don't understand the use of initialState.

Another thing, regarding the React docs, I think you misread them. The link you provided only offers an example with useState and some initialState inside, it does not state or show using props as initialState.

No, it clearly has an example of exactly this:

function Counter({initialCount}) {
  const [count, setCount] = useState(initialCount);
  return (
    <>
      Count: {count}
      <button onClick={() => setCount(initialCount)}>Reset</button>
      <button onClick={() => setCount(prevCount => prevCount - 1)}>-</button>
      <button onClick={() => setCount(prevCount => prevCount + 1)}>+</button>
    </>
  );
}

This is what actual docs say about it: “There should be a single “source of truth” for any data that changes in a React application.” and “If something can be derived from either props or state, it probably shouldn’t be in the state.”

Yes, again, then you shouldn't be using initialState in that use case. If the initialState can change then it's not and initialState, it's a reactive prop.

I'm not sure what you're trying to prove with the links provided. My comment was that initialState isn't something bad as long as you know how to use it and placing a blanket "never use initialState" only avoids properly understanding the use case.

Even within the last link you provide the author goes into detail on proper use cases and states:

In general, props should be avoided in the initial state unless you only need them to initialize the internal state of the component and either props are never updated or the component should not react to their updates.

Let's look at the example above - we pass the initialValue prop, which is used to initialize the internal state of the ChildComponent and is never changed.

This is perfectly fine, but it is an exception rather than the general rule.

The real problem occurs when the initialValue in the ParentComponent can be changed

Which exactly reiterates my point - understanding React's lifecycle and the use of initialState.

Juraj Pavlović • Nov 17 '21

Using initial state for generic components is perfectly fine. My initial point was one for outside generic components. That's why you see examples provided in the last comment. Will make an edit explaining in it in more detail.

Starver • Nov 17 '21

This was a very informative article and also this thread was a good discussions. What i get from this thread is that props can be used as initial state only when we are sure that the prop is not going to change or the component is not bothered by the change in prop.

Juraj Pavlović • Nov 17 '21

Thats what this amazing community is for. Contributing even more to a post with a discussion makes each of us learn more. The Props in Initial State heading and content have been edited to provide more details. It should be more clear now when is it good and when it isn't to use such a thing. I kindly ask you to check it out.

Christian Kozalla • Nov 20 '21

I agree that derived state from props isn't a bad practice. Then again, writing a component like that may not always be intentional, but due to lack of experience with React. It is a common beginner's mistake. I think, that's why it is considered a bad practice altogether.

Your two examples of the Counter are nice, however they do not behave the same. First one is managing its on internal state, the second Counter is only a presentational component. Image you want many independent counters on the page, so you would need more than one instance of Counter.

I've recently read "Writing Resilient Components" by Dan Abramov

However, a common mistake when learning React is to copy props into state. (Example with class component) This might seem more intuitive at first if you used classes outside of React. However, by copying a prop into state you’re ignoring all updates to it. In the rare case that this behavior is intentional, make sure to call that prop initialColor or defaultColor to clarify that changes to it are ignored.

overreacted.io/writing-resilient-c...

If initialCount prop of your first Counter would change, would the instance update/re-render or not (like in Dan's example)?

Mark Erikson • Nov 15 '21

Yeah, I covered the "Context vs Redux" question in much more detail in my post Why React Context is Not a "State Management" Tool (and Why It Doesn't Replace Redux).

Also, since I did that "When to Reach for Redux" podcast interview, we've added a new RTK Query data fetching and caching API to Redux Toolkit, which solves the same use case as React Query, Apollo, Urql, and SWR. I've also added a new section on how to use RTK Query to the "Redux Essentials" tutorial as well.

Juraj Pavlović • Nov 15 '21

Your articles about Redux and how Redux compares to Context are incredibly thorough and well written. I specially loved them and mentioned one of them in the resources. Thank you so much for stopping by!

Reotech • Nov 15 '21

So if we don't use index as key..please what should we use as key then?

Juraj Pavlović • Nov 15 '21 • Edited

Good question! One major thing to avoid alongside index is random keys such as uuid. A proper key should be a unique identifier from the data object you are passing. Check out the following example:

Mike Robinson • Nov 16 '21 • Edited

Instead of uuid, you could use this function that just returns an incrementing integer (guaranteed to be unique and no external dependencies):

var getNextKey = (function(n) {
  return () => ++n;
}(0));

James Thomson • Nov 16 '21

One major thing to avoid alongside index is random keys such as uuid
All you need to avoid is setting the key an each render. It's perfectly fine to use a uuid if you define it as part of the object being consumed and that uuid persists.

Juraj Pavlović • Nov 16 '21

Indeed, if the uuid is a part of the object, then of course it is fine to use it. That way it is not a random key like I've mentioned.

Reotech • Nov 15 '21

Thank you for this

Cédric Teyton • Feb 16 '23

Hi there, FYI we've added your blog post on a GitHub project gathering content about best coding practices :)
github.com/promyze/best-coding-pra...

Juraj Pavlović • Nov 16 '21

Now this example makes me understand your initial point more clearly. In such cases I agree with you completely. When making such generic components to be used throughout the app, it makes sense. Thank you.

Reotech • Nov 15 '21

True true..thank you

Chaz Gatian • Nov 19 '21

Do you mind providing an example of what you mean by "Declaring components inside of components"?

Good article, thank you.

Juraj Pavlović • Nov 22 '21

Of course I don't mind, here is an example of it:

Keep in mind that this is a bad practice

hasn-ab • Dec 1 '21

Agreed completely.
Another case could be when we know for sure that props may not change.

tanth1993 • Nov 16 '21

I think Using props in Initial State in case we need to update data from props. ex: a input field needs defaultValue is prop then we change it and the new data in state.

View full discussion (25 comments)

DEV Community

Bad ReactJs practices to avoid

Using state for too much stuff

Using Redux when you don't need it

Declaring components inside of components

Using props in Initial State (in some cases)

Using index as a Key

Using the spread operator frequently

Not using useEffect, useMemo & useCallback dependencies

Doing premature optimizations

Badly declaring TypeScript types

Conclusion

Top comments (25)

Read next

Introducing svelte-bundle

Login/cadastro com firebase + Vue JS #PASSO 1️⃣

I created a video tutorial on how to make a responsive sidebar using CSS and vanilla Javascript

Angular vs. ReactJS: Which One Should You Pick for Your Next Project?