React component code smells

Tram Ho

Original article: https://antongunnarsson.com/react-component-code-smells/

What is Code Smell? A code smell is something that can indicate a problem deeper within the code, but not necessarily a bug. Read more on Wikipedia

1. Too many props

A component with too many props is a sign that the component should be broken up.

So how much is too much? That depends .

You can see a component with 20 props and be satisfied it’s still working fine, now you want to add one more to the already long list of props, there are a few things to consider:

Does this component do many things?

Like functions, components should only do one thing well, so always check if it’s possible to split the component into multiple child components?

Should I use composition?

A good but often overlooked pattern is to create compose components instead of handling all the logic within it. Suppose we have a component like this:

Looking at the props of this component, we can see that they are all related to the functionality of the component, but there is still room to improve this by passing in some child components instead:

Now we’ve made sure that the ApplicationForm only handles onSubmit and onCancel . Child components can handle anything related to their part of the bigger picture. This is also a great opportunity to use React Context for parent-child component communication.

Read more about compound components in React .

Am I sending too many config props?

In some cases, it’s a good idea to group props together into an object, for example to make swapping this configuration easier.

All props except data can be considered as config. In these cases, you should change the Grid component so that it receives an option that options the above configs.

2. Incompatible props

Avoid sending props that are incompatible with each other.

For example we create a generic <Input /> component that aims to handle type=text , but after a while we add functionality to it with type=tel . The implementation could be as follows:

The problem is that isPhoneNumberInput and autoCapitalize don’t match. We can’t capitalize phone numbers.

In this case, the solution is probably to split it into many smaller components. If we have some logic to share, let’s move it to a custom hook :

While this example is a bit convoluted, finding props that are not compatible with each other is often a good indication that you should check if the component needs to be broken down.

3. Copying props into state

Don’t block the flow of data by copying props into state.

By putting props text in the initialValue of useState causes the component to ignore updates of props text . If the props change, the component still renders with the first value, for most props this is unexpected behavior, which can make the component more prone to errors.

A more practical example of this is when we want to get some new value from a value, and especially if this requires some slow computation. In the example below, we run the slowFormatText function to format props text , which takes a long time to execute.

A better way to deal with this is to use the useMemo hook to remember the results:

Now slowlyFormatText runs only when the text changes and we don’t block component update.

Sometimes we still need the prop where the changes are ignored, for example the color picker has given options, but when the user changes it, we don’t want to change it immediately, just save it temporarily. In this case it is perfectly fine to copy the prop into the state, but to represent this behavior of the user we can split it to initialColor or defaultColor

Further reading: Writing resilient components by Dan Abramov .

4. Returning JSX from functions

Don’t return JSX from functions inside the component.

This is a pattern that has largely disappeared as function components become more common, but I still come across it from time to time.

While this may be okay at first, it makes it difficult to reason about the code, is discouraged, and should be avoided. To solve this problem you should split these into subcomponents instead.

Remember that when you create a new component, you don’t need to split the new file. Sometimes you should keep multiple components in the same file if they are tightly coupled.

5. Multiple booleans for state

Avoid using multiple state booleans to represent the state of the component

When writing a component and extending its functionality, you have multiple states to indicate which state the component is in. For example:

Although it’s technically working fine, it’s hard to reason about the state of the component, it can go wrong. We can fall into impossible state if we accidentally set both isLoading and isFinished to true at the same time.

A better way to manage that state is to use enum . In other languages ​​enum is a way to declare a set of constant values, since enums don’t exist in javascript we can use string instead.

Doing it this way we removed the impossible state and made the component more understandable. If you use TypeScript, you can declare it like this:

6. Too many useState

Avoid using too much useState in the component.

A component with too much useState is like doing too many things in it, it is better to split it into multiple components, however there will be complex components that need a lot of state.

We have reset function to reset states, selectItem function to update some state. Both of these functions pretty much use setState to do the task. Now, if we have more actions that have to update the state, this becomes difficult to keep error free in the long run. In these cases, it is beneficial to manage the state with a useReducer

With the use of useReducer we have encapsulated the state management logic and moved the complexity out of the component. This makes it easier to understand what’s going on, we have separate UI and logic.

Both useState and useReducer have their pros and cons for different use cases, one of my favorites with reducers is Kent C. Dodds’ state reducer pattern.

7. Large useEffect

Avoid using large useEffect and do multiple things. It makes the code difficult to find bugs, keep updating.

One mistake I made was putting too many things in one useEffect .

useEffect is not very big but does a lot of work. If the id changes then fetch the post, if unlisted changes then setVisibility, but as long as one of the two dependencies changes, both will work.

For easy tracking we should split into two useEffect with separate dependencies

This way we reduce the complexity of the component, make it easier to guess the logic, and reduce the risk of errors.

8. Wrapping up

Alright, that’s it for now! Remember that the foregoing is not a rule but an indication that something may be “wrong”. You will definitely encounter the above situations and want to make corrections.

Would you like feedback or suggestions for other code smells ? Find me on Twitter!

Share the news now

Source : Viblo