React lifecycle with react hooks useEffect

Question

I am currently changing my app files from class with lifecycle events such as componentDidMount to functions with useEffect hooks.

For most files I haven't seen any issues but on the below, I am getting a performance issue, where the app freezes. Zero errors or warnings in the console but my machine and Chrome increases memory used on this tab.

What have I missed?

Old Class based file code

listener: any;

componentDidMount() {
  const { firebase } = this.props;
  this.listener = firebase.onAuthUserListener(
    (authUser: any) => {
      localStorage.setItem('authUser', JSON.stringify(authUser));
      this.setState({ authUser });
    },
    () => {
      localStorage.removeItem('authUser');
      this.setState({ authUser: null });
    }
  );
}

componentWillUnmount() {
  this.listener();
}

New with hooks (causing performance issues)

const listener = () => {
  firebase.onAuthUserListener(
    (authUser: any) => {
      localStorage.setItem('authUser', JSON.stringify(authUser));
      setState(authUser);
    },
    () => {
      localStorage.removeItem('authUser');
      setState(null);
    }
  );
};

useEffect(() => {
  listener();
  return () => {
    listener();
  };
});

It may also be worth noting that I am using TypeScript with React.

Whoops, I didn't. Bad case of copy/paste from my original code. — Darren
– Darren, Commented Oct 19, 2019 at 3:32

Joseph D. · Accepted Answer · 2019-10-19 04:35:30Z

2

onAuthUserListener returns a function to unsubscribe. This should be used when component unmounts.

In your code you are not returning the unsubscribe function.

const listener = () => {
  firebase.onAuthUserListener(..)    // problem here no return
}

So under the useEffect you should assign it properly and use it in useEffect's return.

const [user, setUser] = React.useState(null);

useEffect(
  () => {
    //             v------ proper assignment.
    const listener = firebase.onAuthUserListener(
      (authUser: any) => {
        localStorage.setItem('authUser', JSON.stringify(authUser));
        setUser(authUser);
      },
      () => {
        localStorage.removeItem('authUser');
        setUser(null);
      }
    );

    return () => listener();
  }
  , [] // no deps
);

edited Oct 19, 2019 at 4:35

answered Oct 19, 2019 at 3:39

Joseph D.

12.2k4 gold badges40 silver badges72 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Darren Over a year ago

Thanks Joseph. This is what worked best for me. However, I have added firebase to the dependancies.

Rafael Mora · Accepted Answer · 2019-10-19 03:40:07Z

1

Here might be the error:

useEffect(() => {
  listener();
  return () => {
    listener();  <--- here
  };
});

The question is, what state does need to change in order to trigger a listener(); call?

I think this:

const listener = () => {
  firebase.onAuthUserListener(
    (authUser: any) => {
      localStorage.setItem('authUser', JSON.stringify(authUser));
      setState(authUser);
    },
    () => {
      localStorage.removeItem('authUser');
      setState(null);
    }
  );
};

Should be transformed into a React Hook. Cheers!

answered Oct 19, 2019 at 3:40

Rafael Mora

1,2412 gold badges16 silver badges25 bronze badges

Comments

Agney · Accepted Answer · 2019-10-19 03:27:09Z

0

useEffect needs a dependency array as second argument, otherwise it keeps running on each render. Since you are updating state, this just keeps repeating

useEffect(() => {
  listener();
  return () => {
    listener();
  };
}, []);

Docs

You can use tools like Runaway Effects or ESLint for static analysis for figuring these out beforehand.

answered Oct 19, 2019 at 3:27

Agney

19.3k7 gold badges60 silver badges76 bronze badges

4 Comments

Darren Over a year ago

Thank you. That was my original thought, however I get thrown this warning

Line 37:8:  React Hook useEffect has a missing dependency: 'listener'. Either include it or remove the dependency array  react-hooks/exhaustive-deps

Darren Over a year ago

If I add listener to the dependancy I get

The 'listener' function makes the dependencies of useEffect Hook (at line 37) change on every render. Move it inside the useEffect callback. Alternatively, wrap the 'listener' definition into its own useCallback() Hook  react-hooks/exhaustive-deps

Agney Over a year ago

ESLint figures that everything used inside useEffect is something that can change and introduce bugs, just add listener to the deps array, you can find more info in docs

Darren Over a year ago

Yes, that is what I have decided upon. The reason was listener was being reused and pulled from another file. Adding it inside of useEffect and removing the dependancy seems to have worked. Just testing now.

Collectives™ on Stack Overflow

React lifecycle with react hooks useEffect

3 Answers 3

1 Comment

Comments

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

Comments

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related