setState hook inside useEffect can cause unavoidable warning Can't perform a React state update

[istarkov]

BUG

What is the current behavior?

Example: https://codesandbox.io/s/6y1x2zr21n clicking on OK button cause Warning: Can't perform a React state update on an unmounted component.

The problem that unsubscribe is called during B event setVisible(v => false); call, see logs:

SET VISIBLE BEFORE 
UNSUBSCRIBE 
Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.
    in Child (created by Holder)
SET VISIBLE AFTER 

In our case we have this even without RAF call, but on transitionend DOM event.
(It's occurred randomly and rare in our codebase as transitionend event should be called exactly at needed time, but example showed what happens)
Seems like it occurred only if you have a setState call during useEffect callback like setRefresh(v => v + 1); (inside provided code) (after rewriting our codebase to avoid setState calls in useEffect the error has gone)

Code

import React from "react";
import ReactDOM from "react-dom";
import mitt from "mitt";

const emitter = mitt();

const Child = () => {
  const [visible, setVisible] = React.useReducer((s, a) => a, true);
  React.useEffect(() => {
    const handle = () => {
      console.log("SET VISIBLE BEFORE");
      setVisible(v => false); // <--- THIS CALL CAUSES UNSUBSCRIBE AND WARNING ABOUT STATE
      console.log("SET VISIBLE AFTER");
    };
    emitter.on("B", handle);
    return () => {
      console.log("UNSUBSCRIBE");
      emitter.off("B", handle);
    };
  }, []);

  return <div>{visible && <h1>CHILD TEXT</h1>}</div>;
};

const Holder = () => {
  const [refresh, setRefresh] = React.useState(0);
  const visible = React.useRef(true);
  React.useEffect(() => {
    if (refresh === 1) {
      visible.current = false;
      setRefresh(v => v + 1); // <--- This state change from effect caused problems
    }
    const handle = () => {
      setRefresh(v => v + 1);
    };
    emitter.on("A", handle);
    return () => {
      emitter.off("A", handle);
    };
  });

  return <div>{visible.current && <Child />}</div>;
};

function App() {
  return (
    <div>
      <Holder />
      <button
        onClick={() => {
          emitter.emit("A", {});

          requestAnimationFrame(() => {
            emitter.emit("B", {});
          });
        }}
      >
        OK
      </button>
    </div>
  );
}

const rootElement = document.getElementById("root");
ReactDOM.render(<App />, rootElement);

What is the expected behavior?

Do not provide warning if unsubscription is called during "setState" call.

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

React 16.7.0-alpha.2

[Voronar]

  React.useEffect(() => {
    let unmounted = false; // new stuff here
    const handle = () => {
      console.log("SET VISIBLE BEFORE");
      if (!unmounted) setVisible(v => false); // new stuff here
      console.log("SET VISIBLE AFTER");
    };
    emitter.on("B", handle);
    return () => {
      unmounted = true; // new stuff here
      console.log("UNSUBSCRIBE");
      emitter.off("B", handle);
    };
  }, []);

  React.useEffect(() => {
    let unmounted = false; // new stuff here

    if (refresh === 1) {
      visible.current = false;
      if (!unmounted) setRefresh(v => v + 1); // new stuff here
    }
    const handle = () => {
      if (!unmounted) setRefresh(v => v + 1); // new stuff here
    };
    emitter.on("A", handle);
    return () => {
      unmounted = true; // new stuff here
      emitter.off("A", handle);
    };
  });

This code (or we can prevent whole function execution based on unmonted flag) works without warnings but I think it is not a good solution.

[hijiangtao]

@Voronar I use a manual flag to avoid warnings just like you, too. but I hope that react can improve it internally.

Currently, I think use useLayoutEffect rather than useEffect can also avoid these warnings, too.

:-)

[swissmanu]

the additional tracking using an unmounted variable did not work for me at all... though useLayoutEffect fixed the warning.

[gaearon]

Using useLayoutEffect to avoid a warning is always a wrong fix.

[gaearon]

@swissmanu Post your code (ideally as a runnable CodeSandbox), we can try to help.

[swissmanu]

you're totally right @gaearon (it felt very wrong to me too btw ;) )

after some more in depth inspection of my components life cycles using the profiler i was able to dig down to the underlying problem... et voilà: the warning disappeared.

thx anyway!

[ryansully]

@swissmanu What was your solution?

[swissmanu]

@ryansully a parent component made its child tree unmount because of an async side effect.

the code i tried to fix through useLayoutEffect was in that very child tree. after fixing the problem in the parent, everything went smooth.

sorry to not be able to give you a more satisfying answer 😔

[tranvansang]

My temporary un-tested workaround is

export const useUnmounted = () => {
  const unmounted = useRef(false)
  useEffect(() => () => {
    unmounted.current = true
  }, [])
  return unmounted
}

In your component

const unmounted = useUnmounted()
//some callbacks and statements
if (unmounted.current){
console.log('component has been unmounted')
}else{
}

[misantronic]

My temporary un-tested workaround is

export const useUnmounted = () => {
  const unmounted = useRef(false)
  useEffect(() => () => {
    unmounted.current = true
  }, [])
  return unmounted
}

In your component

const unmounted = useUnmounted()
//some callbacks and statements
if (unmounted.current){
console.log('component has been unmounted')
}else{
}

What is the difference to just using a flag like

let unmounted = false;

React.useEffect(() => () => unmounted = true, []);

[tranvansang]

@misantronic the unmounted variable in your code is different among renders. useRef is an alternative for this.unmounted in class style component.

[gaearon]

Generally a much simpler solution is something like

React.useEffect(() => {
  let didCancel = false;
 
  // ...
  // you can check didCancel
  // before running any setState
  // ...
 
  return () => {
    didCancel = true;
  };
});

[gaearon]

I think didCancel workaround like #14369 (comment) is completely legit in this case because mitt event emitter clones all listeners before dispatching the event (you can check its source code). So removing a listener in the middle of a dispatch would not prevent it from being called. Therefore, it makes sense that all event emitter callbacks using mitt must check a value like didCancel manually. (Redux has a similar design.)

[misantronic]

Generally a much simpler solution is something like

React.useEffect(() => {
  let didCancel = false;
 
  // ...
  // you can check didCancel
  // before running any setState
  // ...
 
  return () => {
    didCancel = true;
  };
});

Perfect solution! Thank you

[tranvansang]

@gaearon your workaround does not work when one wants to do an async action outside of useEffect. This pattern is more common in my project

const request = async () => {
await fetch('some long running task')
if (unmounted) return
//do setState....
}

return <button onClick={request}/>

[gaearon]

Sure, in that case you can keep isMounted in a ref. Just keep in mind this kind of code often hides race conditions. Such as if you click the button two times and the second request comes back before the first one.

[VikR0001]

I'm getting this "unmounted component." warning in my app as well. How is auseEffect hook running, inside a component that has been unmounted?

UPDATE-- oh, I get it. It's a listener running from outside the component -- is that correct?

[VikR0001]

In #14369 (comment), the variable unmounted is declared false at the top of the useEffect hook. Its value is unchanged until component unmount, but is referenced within the useEffect hook. Does this mean the warning is due to the component being unmounted while the code in the useEffect hook is running?

[VikR0001]

Would it make sense to put a ref={ref_checkIfMounted} in the top-level element of the component, and test that, e.g.:

useEffect(() => {
    if (!ref_checkIfMounted.current) return;
    updateNumColumns();
});

I just added it and so far it's thrown no errors.

[gaearon]

@VikR0001 The discussion will be more productive if you link to CodeSandbox that reproduces your problem. We can’t guess anything about your code. So your comments mostly create confusion for future readers. However, if you link to the code then they could become helpful.

[VikR0001]

@gaearon I don't yet know what would cause a useEffect hook to run after the component it is in has been unmounted, so I don't yet know enough to be able to put together a CodeSandbox for this. But, here is the code I have in mind... it seems to be working in my app so far:

function EditorsList(props) {
    const [numColumns, setNumColumns] = useState(1);
    const ref_checkIfMounted = useRef(null);  

    useEffect(() => {
        window.addEventListener("resize", updateNumColumns);
        return function cleanup() {
            window.removeEventListener("resize", this.updateNumColumns);
        };
    });

    function updateNumColumns() {
        if (!ref_checkIfMounted.current) return;
        const newNumColumns = window.innerWidth <= 960 ? 1 : 2;
         if (newNumColumns !== numColumns) {
            setNumColumns(newNumColumns);
        }
   }

     return (
		<>
			<div className={classes.root} key="nav-list" ref={ref_checkIfMounted}>
			//more elements.....
			<div className={classes.bottomSpacer}/>
		</>
	);
};

export default withStyles(styles)(EditorsList);

[gaearon]

The solution in #14369 (comment) is usually the right one. (Or preferably you'd cancel the async work you're doing.)

Note there might also be a legit bug where this warning is unavoidable. I'm tracking that bug in #15057.

I'll lock this issue to prevent further confusion. If you have an avoidable warning like this and #14369 (comment) doesn't solve it, file a new issue with CodeSandbox demo.

[gaearon]

This appears fixed with 0.0.0-db3ae32b8, which is a release candidate for 16.9.