Bug: local variable approach in useEffect failed to achieve "ignoring" check with strict mode in development

[Jerenyaoyelu]

React version: 18.0.0

Steps To Reproduce

1. set up a local variable 'ignore' in useEffect as doc https://beta.reactjs.org/learn/synchronizing-with-effects#how-to-handle-the-effect-firing-twice-in-development introduced.

2. check the console

Link to code example: codepen links https://codesandbox.io/s/keen-hooks-451ugq?file=/src/App.js:227-233

The current behavior

do something is logged twice;

The expected behavior

With the approach above, the first Effect will immediately get cleaned up so its copy of the ignore variable will be set to true. So even though there is an extra request, it won’t affect the state thanks to the if (!ignore) check.

as doc says the do something should be only logged once.

[Jerenyaoyelu]

if I move the local variable to component scope, the ignoring check works.

export default function App() {
  let ignore = false;  // works, if variable is placed here
  useEffect(() => {
    console.log("mounted", ignore);

    async function startFetching() {
      if (!ignore) {
        console.log("do something");
      }
    }

    startFetching();

    return () => {
      ignore = true;
      console.log("cleanup", ignore);
    };
  }, []);
  return (
    <div className="App">something</div>
  );
}

I got:

mounted false
do something 
cleanup true
mounted true

However, this is not following the way the doc introduced.

Did I miss something? Apologize if it is not a bug(react bug or doc mistake).

[eps1lon]

as doc says the do something should be only logged once.

The docs don't say that. They say you should do the cleanup to fix issues caused by running effects twice (e.g. race conditions if a request is made twice but the response comes back out-of-order).

The logging will still happen twice since the effect will still fire twice. Cleanup just makes sure the user doesn't see the double effect.

If a console.log is user-observable then your cleanup needs to make sure it can clean it up. Since the browser console is append-only that's not possible.

But the browser console is usually only for development. Maybe you can explain in more detail why the console is relevant to the user?

[Jerenyaoyelu]

console here is just an example indicating there is some logic handling. I understood the cleanup is to fix issues caused by double firing.

My point is that if the logic handling under the local variable check block(if (!ignore)) still will run twice, why do we bother to put a local variable check? It is a fake check because the !ignore will be true in both fires.

Follow the doc, it looks like the local variable check at least can manage one skip of the do something code. Otherwise, it is meaningless to have such local variable and such condition check if (!ignore). We can simply do cleanup like the following:

export default function App() {
  useEffect(() => {
    async function startFetching() {
      console.log("do something");
    }

    startFetching();

    return () => {
      console.log("undo something");
    };
  }, []);
  return (
    <div className="App">something</div>
  );
}

Not sure if I explain my point clearly 🤣 .

[eps1lon]

I understood the cleanup is to fix issues caused by double firing.

Yes and it does.

My point is that if the logic handling under the local variable check block(if (!ignore)) still will run twice, why do we bother to put a local variable check? It is a fake check because the !ignore will be true in both fires.

The point was not to prevent running twice. Then StrictMode would be pointless. The goal is to have effects be robust against running twice by nudging you to implement correct cleanup logic. For a fetch that usually means preventing a state update from a fetch that was aborted (i.e. ignore is now true).