There’s usually more than one way to code a thing in React. And while it’s possible to create the same thing different ways, there may be one or two approaches that technically work “better” than others. I actually run into plenty of examples where the code used to build a React component is technically “correct” but opens up issues that are totally avoidable.

So, let’s look at some of those examples. I’m going to provide three instances of “buggy” React code that technically gets the job done for a particular situation, and ways it can be improved to be more maintainable, resilient, and ultimately functional.

This article assumes some knowledge of React hooks. It isn’t an introduction to hooks—you can find a good introduction from Kingsley Silas on CSS Tricks, or take a look at the React docs to get acquainted with them. We also won’t be looking at any of that exciting new stuff coming up in React 18. Instead, we’re going to look at some subtle problems that won’t completely break your application, but might creep into your codebase and can cause strange or unexpected behavior if you’re not careful.

Buggy code #1: Mutating state and props

It’s a big anti-pattern to mutate state or props in React. Don’t do this!

This is not a revolutionary piece of advice—it’s usually one of the first things you learn if you’re getting started with React. But you might think you can get away with it (because it seems like you can in some cases).

I’m going to show you how bugs might creep into your code if you’re mutating props. Sometimes you’ll want a component that will show a transformed version of some data. Let’s create a parent component that holds a count in state and a button that will increment it. We’ll also make a child component that receives the count via props and shows what the count would look like with 5 added to it.

Here’s a Pen that demonstrates a naïve approach:

This example works. It does what we want it to do: we click the increment button and it adds one to the count. Then the child component is re-rendered to show what the count would look like with 5 added on. We changed the props in the child here and it works fine! Why has everybody been telling us mutating props is so bad?

Well, what if later we refactor the code and need to hold the count in an object? This might happen if we need to store more properties in the same useState hook as our codebase grows larger.

Instead of incrementing the number held in state, we increment the count property of an object held in state. In our child component, we receive the object through props and add to the count property to show what the count would look like if we added 5.

Let’s see how this goes. Try incrementing the state a few times in this pen:

Oh no! Now when we increment the count it seems to add 6 on every click! Why is this happening? The only thing that changed between these two examples is that we used an object instead of a number!

More experienced JavaScript programmers will know that the big difference here is that primitive types such as numbers, booleans and strings are immutable and passed by value, whereas objects are passed by reference.

This means that:

  • If you put a number in a variable, assign another variable to it, then change the second variable, the first variable will not be changed.
  • If you if you put an object in a variable, assign another variable to it, then change the second variable, the first variable will get changed.

When the child component changes a property of the state object, it’s adding 5 to the same object React uses when updating the state. This means that when our increment function fires after a click, React uses the same object after it has been manipulated by our child component, which shows as adding 6 on every click.

The solution

There are multiple ways to avoid these problems. For a situation as simple as this, you could avoid any mutation and express the change in a render function:

function Child({state}){ return <div><p>count + 5 = {state.count + 5} </p></div>
}

However, in a more complicated case, you might need to reuse state.count + 5 multiple times or pass the transformed data to multiple children.

One way to do this is to create a copy of the prop in the child, then transform the properties on the cloned data. There’s a couple of different ways to clone objects in JavaScript with various tradeoffs. You can use object literal and spread syntax:

function Child({state}){
const copy = {...state}; return <div><p>count + 5 = {copy.count + 5} </p></div>
}

But if there are nested objects, they will still reference the old version. Instead, you could convert the object to JSON then immediately parse it:

JSON.parse(JSON.stringify(myobject))

This will work for most simple object types. But if your data uses more exotic types, you might want to use a library. A popular method would be to use lodash’s deepClone. Here’s a Pen that shows a fixed version using object literal and spread syntax to clone the object:

One more option is to use a library like Immutable.js. If you have a rule to only use immutable data structures, you’ll be able to trust that your data won’t get unexpectedly mutated. Here’s one more example using the immutable Map class to represent the state of the counter app:

Buggy code #2: Derived state

Let’s say we have a parent and a child component. They both have useState hooks holding a count. And let’s say the parent passes its state down as prop down to the child, which the child uses to initialize its count.

function Parent(){ const [parentCount,setParentCount] = useState(0); return <div> <p>Parent count: {parentCount}</p> <button onClick={()=>setParentCount(c=>c+1)}>Increment Parent</button> <Child parentCount={parentCount}/> </div>;
} function Child({parentCount}){ const [childCount,setChildCount] = useState(parentCount); return <div> <p>Child count: {childCount}</p> <button onClick={()=>setChildCount(c=>c+1)}>Increment Child</button> </div>;
}

What happens to the child’s state when the parent’s state changes, and the child is re-rendered with different props? Will the child state remain the same or will it change to reflect the new count that was passed to it?

We’re dealing with a function, so the child state should get blown away and replaced right? Wrong! The child’s state trumps the new prop from the parent. After the child component’s state is initialized in the first render, it’s completely independent from any props it receives.

React stores component state for each component in the tree and the state only gets blown away when the component is removed. Otherwise, the state won’t be affected by new props.

Using props to initialize state is called “derived state” and it is a bit of an anti-pattern. It removes the benefit of a component having a single source of truth for its data.

Using the key prop

But what if we have a collection of items we want to edit using the same type of child component, and we want the child to hold a draft of the item we’re editing? We’d need to reset the state of the child component each time we switch items from the collection.

Here’s an example: Let’s write an app where we can write a daily list of five thing’s we’re thankful for each day. We’ll use a parent with state initialized as an empty array which we’re going to fill up with five string statements.

Then we’ll have a a child component with a text input to enter our statement.

We’re about to use a criminal level of over-engineering in our tiny app, but it’s to illustrate a pattern you might need in a more complicated project: We’re going to hold the draft state of the text input in the child component.

Lowering the state to the child component can be a performance optimization to prevent the parent re-rendering when the input state changes. Otherwise the parent component will re-render every time there is a change in the text input.

We’ll also pass down an example statement as a default value for each of the five notes we’ll write.

Here’s a buggy way to do this:

// These are going to be our default values for each of the five notes
// To give the user an idea of what they might write
const ideaList = ["I'm thankful for my friends", "I'm thankful for my family", "I'm thankful for my health", "I'm thankful for my hobbies", "I'm thankful for CSS Tricks Articles"] const maxStatements = 5; function Parent(){ const [list,setList] = useState([]); // Handler function for when the statement is completed // Sets state providing a new array combining the current list and the new item function onStatementComplete(payload){ setList(list=>[...list,payload]); } // Function to reset the list back to an empty array function reset(){ setList([]); } return <div> <h1>Your thankful list</h1> <p>A five point list of things you're thankful for:</p> {/* First we list the statements that have been completed*/} {list.map((item,index)=>{return <p>Item {index+1}: {item}</p>})} {/* If the length of the list is under our max statements length, we render the statement form for the user to enter a new statement. We grab an example statement from the idealist and pass down the onStatementComplete function. Note: This implementation won't work as expected*/} {list.length<maxStatements ? <StatementForm initialStatement={ideaList[list.length]} onStatementComplete={onStatementComplete}/> :<button onClick={reset}>Reset</button> } </div>;
} // Our child StatementForm component This accepts the example statement for it's initial state and the on complete function
function StatementForm({initialStatement,onStatementComplete}){ // We hold the current state of the input, and set the default using initialStatement prop const [statement,setStatement] = useState(initialStatement); return <div> {/*On submit we prevent default and fire the onStatementComplete function received via props*/} <form onSubmit={(e)=>{e.preventDefault(); onStatementComplete(statement)}}> <label htmlFor="statement-input">What are you thankful for today?</label><br/> {/* Our controlled input below*/} <input id="statement-input" onChange={(e)=>setStatement(e.target.value)} value={statement} type="text"/> <input type="submit"/> </form> </div>
}

There’s a problem with this: each time we submit a completed statement, the input incorrectly holds onto the submitted note in the textbox. We want to replace it with an example statement from our list.

Even though we’re passing down a different example string every time, the child remembers the old state and our newer prop is ignored. You could potentially check whether the props have changed on every render in a useEffect, and then reset the state if they have. But that can cause bugs when different parts of your data use the same values and you want to force the child state to reset even though the prop remains the same.

The solution

If you need a child component where the parent needs the ability to reset the child on demand, there is a way to do it: it’s by changing the key prop on the child.

You might have seen this special key prop from when you’re rendering elements based on an array and React throws a warning asking you to provide a key for each element. Changing the key of a child element ensures React creates a brand new version of the element. It’s a way of telling React that you are rendering a conceptually different item using the same component.

Let’s add a key prop to our child component. The value is the index we’re about to fill with our statement:

<StatementForm key={list.length} initialStatement={ideaList[list.length]} onStatementComplte={onStatementComplete}/>

Here’s what this looks like in our list app:

Note the only thing that changed here is that the child component now has a key prop based on the array index we’re about to fill. Yet, the behavior of the component has completely changed.

Now each time we submit and finish writing out statement, the old state in the child component gets thrown away and replaced with the example statement.

Buggy code #3: Stale closure bugs

This is a common issue with React hooks. There’s previously been a CSS-Tricks article about dealing with stale props and states in React’s functional components.

Let’s take a look at a few situations where you might run into trouble. The first crops up is when using useEffect. If we’re doing anything asynchronous inside of useEffect we can get into trouble using old state or props.

Here’s an example. We need to increment a count every second. We set it up on the first render with a useEffect, providing a closure that increments the count as the first argument, and an empty array as the second argument. We’ll give it the empty array as we don’t want React to restart the interval on every render.

function Counter() { let [count, setCount] = useState(0); useEffect(() => { let id = setInterval(() => { setCount(count + 1); }, 1000); return () => clearInterval(id); },[]); return <h1>{count}</h1>;
}

Oh no! The count gets incremented to 1 but never changes after that! Why is this happening?

It’s to do with two things:

Having a look at the MDN docs on closures, we can see:

A closure is the combination of a function and the lexical environment within which that function was declared. This environment consists of any local variables that were in-scope at the time the closure was created.

The “lexical environment” in which our useEffect closure is declared is inside our Counter React component. The local variable we’re interested is count, which is zero at the time of the declaration (the first render).

The problem is, this closure is never declared again. If the count is zero at the time declaration, it will always be zero. Each time the interval fires, it’s running a function that starts with a count of zero and increments it to 1.

So how might we get the function declared again? This is where the second argument of the useEffect call comes in. We thought we were extremely clever only starting off the interval once by using the empty array, but in doing so we shot ourselves in the foot. If we had left out this argument, the closure inside useEffect would get declared again with a new count every time.

The way I like to think about it is that the useEffect dependency array does two things:

  • It will fire the useEffect function when the dependency changes.
  • It will also redeclare the closure with the updated dependency, keeping the closure safe from stale state or props.

In fact, there’s even a lint rule to keep your useEffect instances safe from stale state and props by making sure you add the right dependencies to the second argument.

But we don’t actually want to reset our interval every time the component gets rendered either. How do we solve this problem then?

The solution

Again, there are multiple solutions to our problem here. Let’s start with the easiest: not using the count state at all and instead passing a function into our setState call:

function Counter() { let [count, setCount] = useState(0); useEffect(() => { let id = setInterval(() => { setCount(prevCount => prevCount+ 1); }, 1000); return () => clearInterval(id); },[]); return <h1>{count}</h1>;
}

That was easy. Another option is to use the useRef hook like this to keep a mutable reference of the count:

function Counter() { let [count, setCount] = useState(0); const countRef = useRef(count) function updateCount(newCount){ setCount(newCount); countRef.current = newCount; } useEffect(() => { let id = setInterval(() => { updateCount(countRef.current + 1); }, 1000); return () => clearInterval(id); },[]); return <h1>{count}</h1>;
} ReactDOM.render(<Counter/>,document.getElementById("root"))

To go more in depth on using intervals and hooks you can take a look at this article about creating a useInterval in React by Dan Abramov, who is one of the React core team members. He takes a different route where, instead of holding the count in a ref, he places the entire closure in a ref.

To go more in depth on useEffect you can have a look at his post on useEffect.

More stale closure bugs

But stale closures won’t just appear in useEffect. They can also turn up in event handlers and other closures inside your React components. Let’s have a look at a React component with a stale event handler; we’ll create a scroll progress bar that does the following:

  • increases its width along the screen as the user scrolls
  • starts transparent and becomes more and more opaque as the user scrolls
  • provides the user with a button that randomizes the color of the scroll bar

We’re going to leave the progress bar outside of the React tree and update it in the event handler. Here’s our buggy implementation:

<body>
<div id="root"></div>
<div id="progress"></div>
</body>
function Scroller(){ // We'll hold the scroll position in one state const [scrollPosition, setScrollPosition] = useState(window.scrollY); // And the current color in another const [color,setColor] = useState({r:200,g:100,b:100}); // We assign out scroll listener on the first render useEffect(()=>{ document.addEventListener("scroll",handleScroll); return ()=>{document.removeEventListener("scroll",handleScroll);} },[]); // A function to generate a random color. To make sure the contrast is strong enough // each value has a minimum value of 100 function onColorChange(){ setColor({r:100+Math.random()*155,g:100+Math.random()*155,b:100+Math.random()*155}); } // This function gets called on the scroll event function handleScroll(e){ // First we get the value of how far down we've scrolled const scrollDistance = document.body.scrollTop || document.documentElement.scrollTop; // Now we grab the height of the entire document const documentHeight = document.documentElement.scrollHeight - document.documentElement.clientHeight; // And use these two values to figure out how far down the document we are const percentAlong = (scrollDistance / documentHeight); // And use these two values to figure out how far down the document we are const progress = document.getElementById("progress"); progress.style.width = `${percentAlong*100}%`; // Here's where our bug is. Resetting the color here will mean the color will always // be using the original state and never get updated progress.style.backgroundColor = `rgba(${color.r},${color.g},${color.b},${percentAlong})`; setScrollPosition(percentAlong); } return <div className="scroller" style={{backgroundColor:`rgb(${color.r},${color.g},${color.b})`}}> <button onClick={onColorChange}>Change color</button> <span class="percent">{Math.round(scrollPosition* 100)}%</span> </div>
} ReactDOM.render(<Scroller/>,document.getElementById("root"))

Our bar gets wider and increasingly more opaque as the page scrolls. But if you click the change color button, our randomized colors are not affecting the progress bar. We’re getting this bug because the closure is affected by component state, and this closure is never being re-declared so we only get the original value of the state and no updates.

You can see how setting up closures that call external APIs using React state, or component props might give you grief if you’re not careful.

The solution

Again, there are multiple ways to fix this problem. We could keep the color state in a mutable ref which we could later use in our event handler:

const [color,setColor] = useState({r:200,g:100,b:100});
const colorRef = useRef(color); function onColorChange(){ const newColor = {r:100+Math.random()*155,g:100+Math.random()*155,b:100+Math.random()*155}; setColor(newColor); colorRef.current=newColor; progress.style.backgroundColor = `rgba(${newColor.r},${newColor.g},${newColor.b},${scrollPosition})`;
}

This works well enough but it doesn’t feel ideal. You may need to write code like this if you’re dealing with third-party libraries and you can’t find a way to pull their API into your React tree. But by keeping one of our elements out of the React tree and updating it inside of our event handler, we’re swimming against the tide.

This is a simple fix though, as we’re only dealing with the DOM API. An easy way to refactor this is to include the progress bar in our React tree and render it in JSX allowing it to reference the component’s state. Now we can use the event handling function purely for updating state.

function Scroller(){ const [scrollPosition, setScrollPosition] = useState(window.scrollY); const [color,setColor] = useState({r:200,g:100,b:100}); useEffect(()=>{ document.addEventListener("scroll",handleScroll); return ()=>{document.removeEventListener("scroll",handleScroll);} },[]); function onColorChange(){ const newColor = {r:100+Math.random()*155,g:100+Math.random()*155,b:100+Math.random()*155}; setColor(newColor); } function handleScroll(e){ const scrollDistance = document.body.scrollTop || document.documentElement.scrollTop; const documentHeight = document.documentElement.scrollHeight - document.documentElement.clientHeight; const percentAlong = (scrollDistance / documentHeight); setScrollPosition(percentAlong); } return <> <div class="progress" id="progress" style={{backgroundColor:`rgba(${color.r},${color.g},${color.b},${scrollPosition})`,width: `${scrollPosition*100}%`}}></div> <div className="scroller" style={{backgroundColor:`rgb(${color.r},${color.g},${color.b})`}}> <button onClick={onColorChange}>Change color</button> <span class="percent">{Math.round(scrollPosition * 100)}%</span> </div> </>
}

That feels better. Not only have we removed the chance for our event handler to get stale, we’ve also converted our progress bar into a self contained component which takes advantage of the declarative nature of React.

Also, for a scroll indicator like this, you might not even need JavaScript — have take a look at the up-and-coming @scroll-timeline CSS function or an approach using a gradient from Chris’ book on the greatest CSS tricks!

Wrapping up

We’ve had a look at three different ways you can create bugs in your React applications and some ways to fix them. It can be easy to look at counter examples which follow a happy path and don’t show subtleties in the APIs that might cause problems.

If you still find yourself needing to build a stronger mental model of what your React code is doing, here’s a list of resources which can help:

Similar Posts