Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

State variable not updating in useEffect callback? #14066

Closed
evolutionxbox opened this issue Nov 1, 2018 · 27 comments
Closed

State variable not updating in useEffect callback? #14066

evolutionxbox opened this issue Nov 1, 2018 · 27 comments

Comments

@evolutionxbox
Copy link

evolutionxbox commented Nov 1, 2018

Do you want to request a feature or report a bug?
Bug, maybe? Although thinking about it more makes me think I've misunderstood something.

What is the current behavior?
scroll state variable updates in rendered output but not inside handleScroll event callback.

I reckon this might be due to the fact that when handleScroll is defined scroll is 0, but scroll is defined in the scope above and should be updated when the component is re-rendered.

import React, { useState, useEffect } from "react";

const Scroller = () => {
  const [scroll, setScroll] = useState(window.scrollY);

  const handleScroll = () => {
    console.log(scroll); // scroll is always 0
    setScroll(window.scrollY);
  };

  useEffect(() => {
    window.addEventListener("scroll", handleScroll);
    return () => window.removeEventListener("scroll", handleScroll);
  }, []); // runs once

  return <div id="scroll">{scroll}</div>; // scroll is correct
};

export default Scroller;

Please have a play:
Edit yv8vwo44px

What is the expected behavior?
scroll to be updated inside handleScroll event callback and in render output.

Which versions of React, and which browser / OS are affected by this issue?

chrome        70.0.3538.77
react         16.7.0-alpha.0 - next
react-dom     16.7.0-alpha.0 - next
@AyWa
Copy link

AyWa commented Nov 2, 2018

@evolutionxbox

It is not a bug, but the normal behavior from the doc. You can check this issue for more detail #14042

In short, if you are using the second parameter [] then you need to put all the variable (state or not) that you rely on.

So in your case, if you want to have access to the scroll state variable then you need to put [scroll] :)

Obviously you maybe don't want to do that, because you don't want to create / remove event listener everytimes.

So the other way is to use an other effect, that will be responsible to modifying or getting the value. useReducer is one way. (see the other issue for code example)

I hope it is clear :)

@gaearon
Copy link
Collaborator

gaearon commented Nov 2, 2018

Yep that's right. We might offer a more convenient way in the future.

The rule of thumb is that if you use a variable in useEffect, you must declare it in the useEffect dependency array (or omit the array entirely). So in your example handleScroll should be in the array.

It's true that this would cause it to re-subscribe more often. There is a way to avoid it but it's not very convenient and might cause other issues in the future. We plan to add a better way later but it will take some time to implement.

@evolutionxbox
Copy link
Author

This did help. Thanks @AyWa and @gaearon

@gaearon
Copy link
Collaborator

gaearon commented Nov 5, 2018

I filed #14099 to track the root problem.

@gpietro
Copy link

gpietro commented Feb 14, 2019

I had same kind of problem when trying to add an eventListener to a component. For now, could be a better approach to stay with classes in these circumstances?

@falconvk
Copy link

@gpietro It seems like it. Current behavior just feels broken and the "workarounds" to achieve something as simple as this are just not worth it.

@TidyIQ
Copy link

TidyIQ commented Apr 25, 2019

I'm having the same problem so I won't open a new issue for it. Hopefully someone here can help me out with it.

Here's the code:

  let labelWidth = 0
  React.useEffect(() => {
    labelWidth = labelRef.current!.offsetWidth;
    console.log("After mount:", labelWidth);
  }, [labelWidth]);
  console.log("Current value:", labelWidth);

And the console output on initial render of the whole app:

Current value: 0
After mount: 120

Then when I open the component that uses the labelWidth value the entire console becomes:

Current value: 0
After mount: 120
Current value: 0

useEffect isn't updating the labelWidth property after mount. I've followed the instructions above so I'm not sure what the issue is. Any help?

@AyWa
Copy link

AyWa commented Apr 25, 2019

@TidyIQ

I think you want to use useState if you want it to be consistent.
Something like:

const [labelWidth, setLabelWidth] = useState(0);
  React.useEffect(() => {
    setLabelWidth(labelRef.current!.offsetWidth);
    console.log("After mount:", labelWidth);
  }, [labelWidth]);
  console.log("Current value:", labelWidth);

@crowicked
Copy link

crowicked commented Aug 7, 2019

I'm having the same problem so I won't open a new issue for it. Hopefully someone here can help me out with it.

Here's the code:

  let labelWidth = 0
  React.useEffect(() => {
    labelWidth = labelRef.current!.offsetWidth;
    console.log("After mount:", labelWidth);
  }, [labelWidth]);
  console.log("Current value:", labelWidth);

And the console output on initial render of the whole app:

Current value: 0
After mount: 120

Then when I open the component that uses the labelWidth value the entire console becomes:

Current value: 0
After mount: 120
Current value: 0

useEffect isn't updating the labelWidth property after mount. I've followed the instructions above so I'm not sure what the issue is. Any help?

@TidyIQ : had a similar issue recently, but using two hooks that shared a global and changing the value of it in each one. The value change/assignment is not persisted across useEffect invocations (there's even a warning that says that), probably related to memoization or caching/storing of the initial variable value when the hook function is created. The warning suggested using useRef to store the value used across useEffect calls but couldn't get it to work. Ended up ditching useEffect completely, used plain functions to set input/component values on every render.

Given useEffect fires after the render, my issue was related to a slider component onChange firing to adjust for the accepted value range, setting the wrong state and ruining the workflow by (unwanted) firing another re-render.

Is there a recommended way to update state or globals, using hooks, before the render happens?

@attilaaronnagy
Copy link

if you need a real unmount, not the crapy fake one that you find in all the examples and a one that has the correct state ... add this to your hook function :D 100% fix :D

In you hook function:

  return (
    <div>
      <RunOnUnmount onUnmount={__your_unmount_function__} />
    </div>
  );
class RunOnUnmount extends React.Component {
  componentWillUnmount() {
      this.props.onUnmount();
  }
  render() {
    return null;
  }
}

@ghost
Copy link

ghost commented Sep 20, 2019

almost same problem.

I am updating the global state using context api, but when I am calling global state in useEffect() it still retreive the initial value, not updated value. here I used [], as second parameter for useEffect function. when I remove it, it execute all code withing useEffect function as unlimited.

@jasollien
Copy link

Do you want to request a feature or report a bug?
Bug, maybe? Although thinking about it more makes me think I've misunderstood something.

What is the current behavior?
scroll state variable updates in rendered output but not inside handleScroll event callback.

I reckon this might be due to the fact that when handleScroll is defined scroll is 0, but scroll is defined in the scope above and should be updated when the component is re-rendered.

import React, { useState, useEffect } from "react";

const Scroller = () => {
  const [scroll, setScroll] = useState(window.scrollY);

  const handleScroll = () => {
    console.log(scroll); // scroll is always 0
    setScroll(window.scrollY);
  };

  useEffect(() => {
    window.addEventListener("scroll", handleScroll);
    return () => window.removeEventListener("scroll", handleScroll);
  }, []); // runs once

  return <div id="scroll">{scroll}</div>; // scroll is correct
};

export default Scroller;

Please have a play:
Edit yv8vwo44px

What is the expected behavior?
scroll to be updated inside handleScroll event callback and in render output.

Which versions of React, and which browser / OS are affected by this issue?

chrome        70.0.3538.77
react         16.7.0-alpha.0 - next
react-dom     16.7.0-alpha.0 - next

I had the exact same problem in my branch. I solved it similar to this, but I don't think it is nice at all. Looking forward to the next update.

  const handleScroll = () => {
    setScroll((previousScroll) => {
      console.log(previousScroll);
      return window.scrollY;
    });
  };

@Abhay2012
Copy link

Abhay2012 commented Nov 24, 2019

I'm having a similar issue

import { useState, useEffect } from "react";

const counter = ({ count, speed }) => {
    const [inc, setInc] = useState(0);

    useEffect(() => {

        const counterInterval = setInterval(() => {
            if(inc < count){
                setInc(inc + 1);
            }else{
                clearInterval(counterInterval);
            }
        }, speed);

    }, [count]);

    return inc;
}

export default counter;` 

In the above code, I want to call useEffect callback only when count (coming from props) updates and I'm increasing inc in setInterval callback till it becomes equal to count.

Now the issue is setInc is incrementing the value of inc but in useEffect's and setInterval's callback, I'm always getting 0, which I think should get updated as inc is in the closure these callbacks

@evolutionxbox
Copy link
Author

You’re not including all dependencies in the array. Try putting inc in the useEffect array.

@Abhay2012
Copy link

Abhay2012 commented Nov 24, 2019

You’re not including all dependencies in the array. Try putting inc in the useEffect array.

I can't do that, as I've setInterval in useEffect, if I put inc in array it will be infinite loop of setIntervals

@evolutionxbox
Copy link
Author

You might need to rethink what it does then. If you don’t include inc in the array it won’t update.

Why not use a setTimeout instead?

Also I’m not sure it’s worth putting a prop as a dependency of the useEffect.

@Abhay2012
Copy link

It's a counter, it takes a value (max value) in props, then initializes inc with 0 and keeps increment inc till inc become equals to count, so I can't use setTimeout ( as it will execute call back first time only ) in my case

I've put count in array because count value is coming from API call in parent component so count is initially 0, which I can solve by loading counter component only after getting data from server, So count in array is not an issue I can remove its dependency, my issue is why inc is not getting update in setInterval's callback

Like if I use class component I can access updated inc from this.state.inc and will get the updated value but in case of functional component I'm not able to achieve the same

@evolutionxbox
Copy link
Author

It’s likely that when you wrote the class component it had a bug in it which you didn’t notice.

The functional hook version is forcing you to face that bug first, not later.

Try using a normal variable inside the useEffect rather than using state?

@Abhay2012
Copy link

Here is the class version

class Counter extends React.Component{
    constructor(props){
        super(props);
        this.state = {
            inc: 0
        }
    }

    componentDidMount(){
        const { count, speed = 100 } = this.props;

        const counterInterval = setInterval(() => {
            let { inc } = this.state;
  
            if(inc < count){
                this.setState({ inc: inc + 1 });
            }else{
                clearInterval(counterInterval);
            }

        }, speed)
    }

    render(){
        return this.state.inc;
    }

}

Like above in componentDidMount I'm able to get updated value of inc, I want the same thing using functional component, can you help to achieve the same using functional component

I can't use a normal variable, as react is not going to render Counter component on it's update

@evolutionxbox
Copy link
Author

@gaearon any comments?

@malixsys
Copy link

include inc in array and use React.useRef to check if count has changed, if it has, destroy the previous interval and recreate it...?

@ayal
Copy link

ayal commented Jan 23, 2020

@AyWa 's solution worked for me (i.e using a reducer)

However I was able to solve a similar issue (subscribing once in useEffect and updating state in callback) while still using useState(), alongside with the "function updater" option of the state update function

as mentioned here:
https://reactjs.org/docs/hooks-reference.html#functional-updates

@evolutionxbox
Copy link
Author

evolutionxbox commented Feb 26, 2020

Swapping setInterval for setTimeout did the job. I'm also using the "clean up" function in the useEffect to remove the timeout when the component updates.

import { useState, useEffect } from "react";

const counter = ({ count, speed }) => {
  const [inc, setInc] = useState(0);

  useEffect(() => {
    const counterTimeout = setTimeout(() => {
      if (inc < count) {
        setInc(inc + 1);
      }
    }, speed);

    return () => clearTimeout(counterTimeout);
  }, [inc]);

  return inc;
};

export default counter;

See live demo: https://codesandbox.io/s/brave-kalam-j7xkb

@rahulsethione
Copy link

In case if you see stale or older values of state or props in useEffect callback or in event handlers, React suggests you to use refs keep the update values. This behavior is not due to React but its due to what values of a variable a javascript function receive at run-time (lexical scope and closures). React devs have tried to address this issue here: https://reactjs.org/docs/hooks-faq.html#why-am-i-seeing-stale-props-or-state-inside-my-function

My opinion, the approach in the above doc solves the issue but seems a bit of a life hack. There are more quirks like this with hooks, so I still would prefer a class-based components for writing components with complex state and business logic.

@arnonate
Copy link

arnonate commented Aug 5, 2021

To anyone coming to this conversation late (like me). I ended up using useRef as suggested above and making a little hook to autosave form data on unmount:

import { useEffect, useRef } from 'react';

import type { FormFields } from './useForm';

type FormState = Readonly<FormFields>;

export type TOptions = {
  formState: FormState;
  isDirty: boolean;
  handleSubmit: (shouldValidate: boolean, formState: FormState) => void;
};

const useAutosave = (options: TOptions): void => {
  const formStateRef = useRef(options.formState);
  const isDirtyRef = useRef(options.isDirty);

  useEffect(() => {
    formStateRef.current = options.formState;
  }, [options.formState]);

  useEffect(() => {
    isDirtyRef.current = options.isDirty;
  }, [options.isDirty]);

  useEffect(
    () => () => {
      if (isDirtyRef.current) options.handleSubmit(false, formStateRef.current);
    },
    [],
  ); // If form is dirty, then save data before tearing down
};

export default useAutosave;

It can then be used in my form component:

useAutosave({ formState, isDirty, handleSubmit });

The key is to useRef and update the ref.current with a useEffect every time the values are updated. This gives you a current representation of the state when the cleanup callback is fired.

@gaearon
Copy link
Collaborator

gaearon commented May 4, 2022

We've submitted a proposal to solve this. Would appreciate your input!

reactjs/rfcs#220

@ghost
Copy link

ghost commented Nov 8, 2022

const element = document.getElementById(
  'myId',
) as HTMLInputElement | null;
if (element) {
  element.scrollIntoView({
    behavior: 'smooth',
  });
}

simonmysun added a commit to simonmysun/indoor-sound-classification-frontend that referenced this issue May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

15 participants