panzerdp / dmitripavlutin.com-comments

7 stars 0 forks source link

/react-hooks-stale-closures/ #44

Open panzerdp opened 3 years ago

panzerdp commented 3 years ago

Written on 10/21/2019 15:09:22

URL: https://dmitripavlutin.com/react-hooks-stale-closures/

OxiBo commented 3 years ago

My problems was that my external API call depends on this updated state value. If I use this => useEffect(function() { console.log(count) // =====> this is going to be the newest value }, [count]); my app performs unnecessary API call while I need this call only on button click

panzerdp commented 3 years ago

@OxiBo Can you provide a gist with the code?

ori-volfo commented 3 years ago

Great article.

Very clear. Very helpful. Thanks!

panzerdp commented 3 years ago

Great article.

Very clear. Very helpful. Thanks!

You're welcome @ori-volfo.

codemonkeynorth commented 3 years ago

Hi, thanks but how would you handle this in your example?

 const alertCount = useCallback(() => {
    setTimeout(() => {
      alert(count)
    }, 3000)
  },[count])

<button onClick={alertCount}>Delayed alert count</button>

click the aysnc button 3 times quickly then click the delayed alert button

alert is still a stale reference of 0 not 3

the only way as per the react docs seems to be to track the current state in a ref?

If you intentionally want to read the latest state from some asynchronous callback, you could keep it in a ref, mutate it, and read from it. https://reactjs.org/docs/hooks-faq.html#why-am-i-seeing-stale-props-or-state-inside-my-function

I can't see a way of getting the latest count for the alert without using a ref.current

panzerdp commented 3 years ago

@codemonkeynorth Yes, most likely you need a Ref.

OxiBo commented 3 years ago

@OxiBo Can you provide a gist with the code?

Here is my little project (refactor one of the udemy courses), I know that this is not the perfect way to implement pagination but I ran into this problems with hooks:

  1. Project - https://github.com/OxiBo/shop-cook;
    1. the component with the problem - https://github.com/OxiBo/shop-cook/blob/master/client/src/components/FavoriteRecipesPagination.js
panzerdp commented 3 years ago

@OxiBo Can you provide a gist with the code?

Here is my little project (refactor one of the udemy courses), I know that this is not the perfect way to implement pagination but I ran into this problems with hooks:

  1. Project - https://github.com/OxiBo/shop-cook;
  2. the component with the problem - https://github.com/OxiBo/shop-cook/blob/master/client/src/components/FavoriteRecipesPagination.js

To perform a side-effect, but skipping the initial render, you need a custom hook:

export function useEffectSkipFirst(...args) {
  const isFirstRender = useRef(true);
  useEffect(() => {
    if (isFirstRender.current) {
      isFirstRender.current = false;
      return;
    }
    return args[0]();
  }, args[1]);
}

Then, inside of your component, useEffectSkipFirst() would perform the fetch when the page state value changes:

const FavoriteRecipesPagination = ({ fetchFavRecipes, pages }) => {
  const [page, setPage] = useState(1);

  const handleClick = (e, n) => {
    e.preventDefault();
    setPage((page) => page + n);
  };

  useEffectSkipFirst(() => {
    fetchFavRecipes(page * favRecipesPerPage - favRecipesPerPage);
  }, [page]);

 // ...

}
DevDaveJ commented 3 years ago

I get what you're saying, I really do. But my anti-virus says that the website you link to for demo (jsitor) is dangerous and full of malicious code

panzerdp commented 3 years ago

I get what you're saying, I really do. But my anti-virus says that the website you link to for demo (jsitor) is dangerous and full of malicious code

@DevDaveJ Can you post a screenshot of what the anti-virus says?

DevDaveJ commented 3 years ago

How do I post a screenshot? @panzerdp

DevDaveJ commented 3 years ago

Anti-Virus-Stale-Closure-Error.jpg

panzerdp commented 3 years ago

@DevDaveJ Thanks for providing the screenshot. I've switched the code samples to run jsfiddlee.

duc168 commented 3 years ago

Thanks again. You give me bunch of knowledge today!

panzerdp commented 3 years ago

Thanks again. You give me bunch of knowledge today!

Thanks @duc168.

MiracleHu commented 3 years ago

Thank you for sharing this awesome tutorial!! I learn a lot from you

panzerdp commented 3 years ago

Thank you for sharing this awesome tutorial!! I learn a lot from you

You're welcome @MiracleHu.

maximus136 commented 2 years ago

Great article. Have one question though. In the below code, where we are adding count as a dependency to always get the updated value within the timer callback:

useEffect(function() {
    const id = setInterval(function log() {
      console.log(`Count is: ${count}`);
    }, 2000);
    return function() {
      clearInterval(id);
    }
  }, [count]);

Since the effect will get executed any time the count state is updated, will it not end up creating new timers each time the value of count changes?

panzerdp commented 2 years ago

Great article. Have one question though. In the below code, where we are adding count as a dependency to always get the updated value within the timer callback:

useEffect(function() {
    const id = setInterval(function log() {
      console.log(`Count is: ${count}`);
    }, 2000);
    return function() {
      clearInterval(id);
    }
  }, [count]);

Since the effect will get executed any time the count state is updated, will it not end up creating new timers each time the value of count changes?

Each time count updates, indeed a new timer starts and the previous active timer is cancelled.

maximus136 commented 2 years ago

Each time count updates, indeed a new timer starts and the previous active time is cancelled. Thanks a lot for the reply! But how does it then maintain the 2s interval, if a new timer is created each time? Does React do anything internally for this?

codemonkeynorth commented 2 years ago

@maximus136 i think you're questioning something this code isn't trying to do there. It just logs every 2 seconds after the count changes.

At a guess, otherwise you would want to have your setInterval call a getCount function and have the getCount wrapped in a useCallback with [count] as the dependency

I'll have to try that out though to confirm

maximus136 commented 2 years ago

@codemonkeynorth Ah! I got what you are saying - the timer only ever restarts when the user changes the value of count. Thanks!

codemonkeynorth commented 2 years ago

@maximus136 yes.

however actually my latter alternative won't work to keep the timer constant since as soon as you press increase then the interval gets cleared anyway (state causes re-render, running the clearInterval function first)

in that case, it would be better to use useRef rather than useState for a count value, to stop a change to state causing a re-render and thus resetting the interval

I've done an example here.. you'll see the timer continues at a 2 second interval no matter how many times and how often you click "Increase Count"

https://codesandbox.io/s/thirsty-lamarr-8hi04?file=/src/App.js

maximus136 commented 2 years ago

@codemonkeynorth good example, and an interesting use case for ref too. Thanks!

baoloc008 commented 2 years ago

Thanks for great article. I have a question. How do we know if a variable was captured or not? Take a look at log() function

function createIncrement(incBy) {
  let value = 0;
  function increment() {
    value += incBy;
    console.log(value);
  }
  function log() {
    console.log(value); // it works
  }

  return [increment, log];
}
function WatchCount() {
  const [count, setCount] = useState(0);
  useEffect(function() {
    setInterval(function log() {
      console.log(count); // it doesn't work
    }, 2000);
  }, []);
  return (
    <div>
      {count}
      <button onClick={() => setCount(count + 1) }>
        Increase
      </button>
    </div>
  );
}
kenanyildiz commented 2 years ago

In my case, I couldn't get the latest value of getText() because of stale the data situation. The difference in this code is, I am binding a function (handleClick) that will run when onClick div and dispatch an event in my system with the latest value of the getText() fn. BUT it is being defined only once and that time getText() value captured (because of closure).

The below code working great. My question is, is this the right way to access latest fresh value? Is there a better/different approach?

export const Label = (props) => {
  const [isTextFromData, setIsTextFromData] = useState(true)
  const [isTextFromWorkflow, setIsTextFromWorkflow] = useState(false)
  const [textFromWorkflow, setTextFromWorkflowToState] = useState('')

  const labelRef = useRef({}) // related ref solution
  const pluginRef = useRef({}) // related ref solution

  // REF 2 SOLUTION - BETTER SOLUTION
  // Always gets the latest version of 'text'
  labelRef.current.value = getText()

  useEffect(() => {
    pluginRef.current.value = {
      registerEvent: {
        handleClick: props.registerEvent({
          key: 'handleClick',
          fn: handleClick, //HERE IS THE DEFINITION THAT WILL WORK ON CLICK
          returnTypes: { text: PluginTypes.string }
        })
      }
    }
  }, [])

  function handleClick() {
    // when I call getText() here, it returns stale data
    return { text: labelRef.current.value }
  }

  function getText() {
    if (isTextFromWorkflow) {
      return textFromWorkflow
    }

    const val = getValue()

    const textColumn = _.get(props, 'settings.config.data.textColumn')
    if (isTextFromData && !_.isUndefined(textColumn)) {
      const formattedValue = getValueWithFormat(textColumn, val)
      return _.isNil(formattedValue) ? '' : formattedValue
    }

    return val
  }

  const onHandleClick = _.get(pluginRef, 'current.value.registerEvent.handleClick', null)

  return (
    <div onClick={onHandleClick}>{getText()}</div>
  )

}
panzerdp commented 2 years ago

@kenanyildiz Yes, using a reference usually is a good way to solve the stale closure problem. If the approach works for you, then use it. In time, if you look later at the code, you might see a better solution.

bassamanator commented 2 years ago

Thank you for another great article! I consider this website my React Bible.

The only thing that I'm confused about is at 3.1. I thought the useEffect cleanup function only ran when the component will unmount. But it seems that it runs everytime the dependencies change? Could you please clairify?

ashok-khanna commented 2 years ago

This was an amazing article that I referred back to a few times. Thank you!!

panzerdp commented 2 years ago

This was an amazing article that I referred back to a few times. Thank you!!

Thank you @ashok-khanna! I'm glad you find it useful.

CLaaay1 commented 1 year ago

I don't understand why in the useEffect example it doesn't log the updated value

function WatchCount() {
  const [count, setCount] = useState(0);
  useEffect(function() {
    const id = setInterval(function log() {
      console.log(`Count is: ${count}`);
    }, 2000);
    return function() {
      clearInterval(id);
    }
  }, [count]);
  return (
    <div>
      {count}
      <button onClick={() => setCount(count + 1) }>
        Increase
      </button>
    </div>
  );
}

what I understand is that every 2 seconds the log function get executed so the ${value} get re evaluated every 2 seconds

panzerdp commented 1 year ago

@CLaaay1 The "updated" value is available only after re-render by assignment of the new state value into count variable (line 2). However, the log function captures the count variable from initial render - thus it's always going to display count as 0 from initial render.

dorji-tshering commented 1 year ago

@bassamanator Hi, yes you are right about clean up function running after the component unmounts. On top of that, the clean up function will also run right before the same useEffect hook is run again due to dependency changes. That's why every time the dependency changes, the useEffect hook is run again and just before that the cleanup function for the previous hook will run to clear any residual side effects from previous hook.

MFQuraishi commented 1 year ago

Thanks for this article. My workaround was simply to add a key to the component where the function with stale closure was passed.

Value of this key preferably should be a value on which a function depends on exactly like dependency arguments in useEffect.

My usecase was that I was passing a callback to a component and that callback developed stale prop values after some time.

TusharShahi commented 1 month ago

Later, even if count increases when the Increase button is clicked, the log() closure called by the timer function every 2 seconds still uses count as 0 from the initial render. -> Won't it be more accurate to say that, the variable count which was referred by first setInterval no more exists, since a different render of WatchCount creates these values afresh. For example if instead of logging the variable count, we were using a variable globalCount also, things would have worked well. So the closure works well referencing the variable best it can, but if the variable does not exist then it has to use the value the variable it held last right?