React: Calling a hook setter recursively in a setTimeout is producing unexpected behaviour.

Created on 15 Jul 2019  路  1Comment  路  Source: facebook/react

Do you want to request a feature or report a bug? a bug, or I'm using wrong 馃槃

What is the current behavior?

const [items, setItems] = useState([]);

function addItems(delay = 0) {
  setItems(items.concat({ timestamp: new Date() }));
  setTimeout(() => {
    addItems();
  }, 1000);
}


//
<button onClick={addItems}>Add items</button>;
{
  items.map((item, index) => 
     <div key={index}> {JSON.stringify(item)} </div>
 );
}

2019-07-15 19 14 58

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:

https://codesandbox.io/s/broken-smoke-5x6g5

What is the expected behavior?
items will contain more and more items after the button is clicked.

Which versions of React, and which browser / OS are affected by this issue? 16.8.6 Did this work in previous versions of React? no

picture tkrugg

Most helpful comment

Okay, I'm somehow week at English, But I'll try my best to explain what's happening here.
First, Let's reconstruct the sample code you provided to this:

const [items, setItems] = useState([]);

function addItem() {
  setItems(items.concat({ timestamp: new Date() }));
}

function handleClick() {
  addItem()
  setTimeout(() => {
    addItem();
  }, 1000);
}

//
<button onClick={addItems}>Add items</button>;
{
  items.map((item, index) => 
     <div key={index}> {JSON.stringify(item)} </div>
 );
}

OK. Now, as Dan said in one of his blog posts, each render has its own state, callback, functions, etc. and if anything is meant to get shared between renders, we should use the useRef hook. It's not some React magic thing; It's simply how js works. If you define a variable in a function and then use it in some other scope (e.g. setTimeout callback), then you only have access to the value of the variable at that point in time when the setTimeout callback gets created, no matter how many times the original function or the timeout callback is getting called. I encourage you to read this awesome post by Dan; It's long but worths reading.

Now let's get back to your code, Here when we are creating the addItem function,

function addItem() {
  setItems(items.concat({ timestamp: new Date() }));
}

this function only has access to whatever the value of the items variable is when it's getting created ( which means an empty array ).
Now when we are setting a timeout inside the handleClick function (which uses this addItem function with empty items in its closure), It doesn't matter anymore how many times we are calling the addItem, because it always points to the original addItem function in the first render pass where the items variable is an empty array. So what happens is that you are setting Items to [].concat(...) every time.

So how we should fix it?

You can consider using the functional form of setItem function which guarantees to pass you the latest value of the items state every time it calls its callback.
The right form of what you were trying to achieve will look like this:

```const [items, setItems] = useState([]);

function addItems(delay = 0) {
setItems((latestItems) => latestItems.concat({ timestamp: new Date() }));
setTimeout(() => {
addItems();
}, 1000);
}

//

>All comments

Okay, I'm somehow week at English, But I'll try my best to explain what's happening here.
First, Let's reconstruct the sample code you provided to this:

const [items, setItems] = useState([]);

function addItem() {
  setItems(items.concat({ timestamp: new Date() }));
}

function handleClick() {
  addItem()
  setTimeout(() => {
    addItem();
  }, 1000);
}

//
<button onClick={addItems}>Add items</button>;
{
  items.map((item, index) => 
     <div key={index}> {JSON.stringify(item)} </div>
 );
}

OK. Now, as Dan said in one of his blog posts, each render has its own state, callback, functions, etc. and if anything is meant to get shared between renders, we should use the useRef hook. It's not some React magic thing; It's simply how js works. If you define a variable in a function and then use it in some other scope (e.g. setTimeout callback), then you only have access to the value of the variable at that point in time when the setTimeout callback gets created, no matter how many times the original function or the timeout callback is getting called. I encourage you to read this awesome post by Dan; It's long but worths reading.

Now let's get back to your code, Here when we are creating the addItem function,

function addItem() {
  setItems(items.concat({ timestamp: new Date() }));
}

this function only has access to whatever the value of the items variable is when it's getting created ( which means an empty array ).
Now when we are setting a timeout inside the handleClick function (which uses this addItem function with empty items in its closure), It doesn't matter anymore how many times we are calling the addItem, because it always points to the original addItem function in the first render pass where the items variable is an empty array. So what happens is that you are setting Items to [].concat(...) every time.

So how we should fix it?

You can consider using the functional form of setItem function which guarantees to pass you the latest value of the items state every time it calls its callback.
The right form of what you were trying to achieve will look like this:

```const [items, setItems] = useState([]);

function addItems(delay = 0) {
setItems((latestItems) => latestItems.concat({ timestamp: new Date() }));
setTimeout(() => {
addItems();
}, 1000);
}

//

Was this page helpful?
0 / 5 - 0 ratings

Related issues

tleunen picture tleunen  路  3Comments
huxiaoqi567 picture huxiaoqi567  路  3Comments
kocokolo picture kocokolo  路  3Comments
h1orz picture h1orz  路  3Comments
jimfb picture jimfb  路  3Comments