3

I have made a working example here: https://codesandbox.io/s/magical-flower-o0gyn?file=/src/App.js

When I click the hide button I want to save the updated data to localstorage:

  1. I click hide on first column: setValueWithCallback runs, sets the callback to a ref & sets the state
  2. useEffect kicks in, calls the callback with the updated data
  3. saveToLocalStorage is called, in a useCallback with data set as a dependency

The problem is at the 3rd step, what gets saved to localstorage is {visible: true} for both. I know if I change this line:

const saveToLocalStorage = useCallback(() => {
  localStorage.setItem(
    `_colProps`,
    JSON.stringify(data.map((e) => ({ id: e.id, visible: e.visible })))
  );
}, [data]);

To this:

const saveToLocalStorage = localStorage.setItem(
  `_colProps`,
  JSON.stringify(data.map((e) => ({ id: e.id, visible: e.visible })))
);

It works, but I cannot get my head around, why it does not with the first. I assume it must be some closure thing, but I don't see it.

If data has already been updated, and useEffect ran the callback, why it is not updated in the dependencies array?. Yes, the example is weird and the 2nd solution is perfectly fine, I just wanted to demonstrate the problem. Thanks for the help!

marchello
  • 2,016
  • 3
  • 29
  • 39
  • Shouldn't this `useCallback(() => {` be `useCallback((data) => {`? You are passing an argument to the callback function but your callback function definition doesn't takes any parameters. – Yousaf Feb 06 '21 at 14:18
  • I don't think you need parameters. If you check the react docs it is used without parameters. – marchello Feb 06 '21 at 14:43
  • So the callback function _magically_ has access to the argument values? In your code example, `data` is the local state of the component, not the argument that you pass when you call `callbackRef.current(data);` – Yousaf Feb 06 '21 at 14:49
  • Sorry, I wasn't clear about that. I don't care about the parameter. Inside of the callback I'm referencing the local state of the component. I just wanted to have a mechanism that first updates the state and when it is done calls the callback (saving to localstorage). Shouldn't the `data` in the dependencies array be already updated when the callback fire? If you click on the other button after the first click, one object is saved correctly, so it is behind a cycle. – marchello Feb 06 '21 at 15:00
  • I might misunderstand how `useCallback` work. I thought when something in the dependencies array change it generates a new function with the most up-to-date values inside of its body. – marchello Feb 06 '21 at 15:09
  • @marchello I thought the same...it's confusing – Timmerz Jun 14 '23 at 18:52

2 Answers2

4

Problem in your code is because of a closure of saveToLocalStorage function over the local state of the component.

Inside the setValueWithCallback function, you save the reference to the saveToLocalStorage function using the callback parameter that is passed to setValueWithCallback function and this is where your problem is.

useCallback hook will update the function reference of saveToLocalStorage function but you do not call that updated function. Instead, you call the function you saved in callbackRef.current which is not the updated function, but an old one which has a closure over the state with value of visible property in both objects set to true.

Solution

You can solve this problem by passing in the data as an argument to the callback function. You already pass the argument when you call callbackRef.current(data) but you don't make use of it inside the saveToLocalStorage function.

Change the saveToLocalStorage to make use of the argument that is passed from inside of the useEffect hook.

const saveToLocalStorage = useCallback((updatedData) => {
    localStorage.setItem(
      `_colProps`,
      JSON.stringify(updatedData.map((e) => ({ id: e.id, visible: e.visible })))
    );
}, []); 

Another way to solve this problem is to get rid of callbackRef and just call the saveToLocalStorage function from inside of the useEffect hook.

useEffect(() => {
    saveToLocalStorage();
}, [data, saveToLocalStorage]);
Yousaf
  • 27,861
  • 6
  • 44
  • 69
  • Thanks, now I see it. Unfortunately, I cannot use the `useEffect` version, because I already have a drag event that fires like a couple hundred times per second setting `data` and that would probably not healthy for localStorage writes. So, I went with the first variant, but I ditched `useCallback` completely, just passing the argument to a regular function. – marchello Feb 06 '21 at 15:46
0

Your data is of array type and react only do shallow comparison when an state update occur, so basically it will check for data reference change not for its value change. That is why, your saveToLocalStorage not re-initiated when data's value changes.

Piyush Rana
  • 631
  • 5
  • 8
  • No, that is not the case because data is getting a new reference each time you click. And If you click the other button after the first, you see the localstorage updating, but still behind one cycle. – marchello Feb 06 '21 at 14:35