Bug: useEffect called twice in Strict Mode

[ag-grid-shared]

Since React 18, useEffect is called twice in Strict Mode when zero dependencies.

This is following on from this tweet:
https://twitter.com/dan_abramov/status/1523652274748559360

The purpose of logging this issue is not to report the problem. The purpose of this issue is to present a scenario where the new behaviour is awkward / difficult to work with, migrating components is not easy, as such I advocate calling this out as a breaking change, and hopefully something that can be turned off in future React versions.

Link to code example:

https://stackblitz.com/edit/react-useeffect-called-twice-scenario

React version: 18

Steps To Reproduce:

Note the UI rendering with / without strict mode is different.

** Problem Pattern 1

UI components from libraries need to maintain their own state & services. They cannot participate with state & services of the hosting application, as then encapsulation of the component is lost. A reliable way is needed to create the state & services of such a component bound to the lifecycle of the component. The new pattern (useEffect() called twice) means the state & services will unnecessarily get created and destroyed twice, along with the useEffect. For a datagrid, this could mean 100,000 rows passed to the grid getting sorted and grouped twice, when it should be once. This has two bad outcomes 1) consumers of the library will observe this and will complain, without knowing it's an intended pattern of React 2) it is bad practice to have different code paths executed in Dev vs Prod modes.

** Problem Pattern 2

For groups of components, where there is shared logic that executes when all components are ready, there is no reliable way to know when all required components are ready, as the components can get destroyed and re-initialised.

In the provided example, note that each React Component works with a Controller class. All the logic is delegated to the Controller class, making the React Component only responsible for DOM operations. This allows the UI to be swapped out with a different rendering engine, while keeping all the logic. This is what AG Grid uses to allow it to use React to render when used with React, and SomethingElse when used with SomethingElse. There are parts of the logic that wait for all Controllers (and associated Components) to be ready before doing certain steps. In React, because the Controllers lifecycle is tied to the useEffect, it means the Controllers are not tied to the Components lifecycle, and resulting in stale references for old Controllers.

In the example code, see ControllersService. This service is what the components register to, and when all components are registered, grid code that was waiting for the components to be ready is notified.

** Breaking Change

This change forces the other non-UI parts of the application to have a fundamental change in how they work. This means React is breaking from the following statement (taken from reactjs.org): "Learn Once, Write Anywhere - We don’t make assumptions about the rest of your technology stack, so you can develop new features in React without rewriting existing code."

**

Yes we could re-write how we do things in AG Grid, however then the bug I am raising is this needs to be documented as a breaking change and React 18 is not backwards compatible.

[gaearon]

this needs to be documented as a breaking change and React 18 is not backwards compatible.

Just to start with this — React 18 is a major release and is not meant to be fully backwards-compatible. Each major release has breaking changes; otherwise we wouldn't mark them as major. The change is documented in the upgrade blog post. It is also marked as a breaking change in the changelog.

I'll respond to the rest later.

[ag-grid-shared]

Just to start with this — React 18 is a major release and is not meant to be fully backwards-compatible. Each major release has breaking changes; otherwise we wouldn't mark them as major. The change is documented in the upgrade blog post. It is also marked as a breaking change in the changelog.

Noted, fair points.

[404answernotfound]

Since React 18, useEffect is called twice in Strict Mode when zero dependencies.

This is following on from this tweet: https://twitter.com/dan_abramov/status/1523652274748559360

The purpose of logging this issue is not to report the problem. The purpose of this issue is to present a scenario where the new behaviour is awkward / difficult to work with, migrating components is not easy, as such I advocate calling this out as a breaking change, and hopefully something that can be turned off in future React versions.

Link to code example:

https://stackblitz.com/edit/react-useeffect-called-twice-scenario

React version: 18

Steps To Reproduce:

Note the UI rendering with / without strict mode is different.

** Problem Pattern 1

UI components from libraries need to maintain their own state & services. They cannot participate with state & services of the hosting application, as then encapsulation of the component is lost. A reliable way is needed to create the state & services of such a component bound to the lifecycle of the component. The new pattern (useEffect() called twice) means the state & services will unnecessarily get created and destroyed twice, along with the useEffect. For a datagrid, this could mean 100,000 rows passed to the grid getting sorted and grouped twice, when it should be once. This has two bad outcomes 1) consumers of the library will observe this and will complain, without knowing it's an intended pattern of React 2) it is bad practice to have different code paths executed in Dev vs Prod modes.

** Problem Pattern 2

For groups of components, where there is shared logic that executes when all components are ready, there is no reliable way to know when all required components are ready, as the components can get destroyed and re-initialised.

In the provided example, note that each React Component works with a Controller class. All the logic is delegated to the Controller class, making the React Component only responsible for DOM operations. This allows the UI to be swapped out with a different rendering engine, while keeping all the logic. This is what AG Grid uses to allow it to use React to render when used with React, and SomethingElse when used with SomethingElse. There are parts of the logic that wait for all Controllers (and associated Components) to be ready before doing certain steps. In React, because the Controllers lifecycle is tied to the useEffect, it means the Controllers are not tied to the Components lifecycle, and resulting in stale references for old Controllers.

In the example code, see ControllersService. This service is what the components register to, and when all components are registered, grid code that was waiting for the components to be ready is notified.

** Breaking Change

This change forces the other non-UI parts of the application to have a fundamental change in how they work. This means React is breaking from the following statement (taken from reactjs.org): "Learn Once, Write Anywhere - We don’t make assumptions about the rest of your technology stack, so you can develop new features in React without rewriting existing code."

**

Yes we could re-write how we do things in AG Grid, however then the bug I am raising is this needs to be documented as a breaking change and React 18 is not backwards compatible.

He answered in this thread, if interested: Answer Link

[mytecor]

My workaround:

import { DependencyList, EffectCallback, useEffect } from 'react'

export default process.env.NODE_ENV === 'production'
	? useEffect
	: (effect: EffectCallback, deps?: DependencyList) => {
			useEffect(() => {
				let ready = true
				let destructor: void | (() => void)

				queueMicrotask(() => {
					if (ready) destructor = effect()
				})

				return () => {
					ready = false
					destructor?.()
				}
			}, deps)
	  }

[cytrowski]

this needs to be documented as a breaking change and React 18 is not backwards compatible.

Just to start with this — React 18 is a major release and is not meant to be fully backwards-compatible. Each major release has breaking changes; otherwise we wouldn't mark them as major. The change is documented in the upgrade blog post. It is also marked as a breaking change in the changelog.

I'll respond to the rest later.

I'll just use the occasion and ping about another docs update I proposed here (with React newcomers in mind): reactjs/react.dev#4650

[gaearon]

I haven't yet looked at the specific sandboxes in this thread in detail, but for people finding this thread I want to note that we've documented the new behavior in detail here:

https://beta.reactjs.org/learn/synchronizing-with-effects#how-to-handle-the-effect-firing-twice-in-development

Hope this is helpful. Sorry some links from there are 404 because other pages are still being written.

[ceolter]

Niall (AG Grid) here - I'm the guy who raised the issue (I mistakenly used ag-grid-shared Github account).

Thanks Dan for the reference. The issue is the recommended approach works for components who's lifecycle work independently of other components.

The problem we face is we have a group of components who's lifecycles work in tandem. So the advice provided doesn't fit onto our design.

I haven't yet looked at the specific sandboxes in this thread in detail, but for people finding this thread I want to note that we've documented the new behavior in detail here:

https://beta.reactjs.org/learn/synchronizing-with-effects#how-to-handle-the-effect-firing-twice-in-development

Hope this is helpful. Sorry some links from there are 404 because other pages are still being written.

[prince-jn]

React intentionally remounts your components in development to help you find bugs like in the last example. The right question isn’t “how to run an Effect once”, but “how to fix my Effect so that it works after remounting”.

If it's for testing purpose... why not having a flag to trigger that behaviour? It doesn't make any sense to have a different behaviour than production.

I understand that you want to force that behaviour to "prepare" users in the future. Are you sure it's a good idea to break something "fundamental" in the library? That's not something users usually like. Not everyone have the time / money to rewrite everything when a new major comes. I'm new to React and it's really complicated to understand how / where to do basic stuff. Plus, it will be a nightmare to understand differences between versions.

Maybe I'm out of topic or don't understand the library well for now but, at least... try to understand a new comer that is lost in debate and new weird behaviour.

The key thing here is that we don't have a clear easy way to do things like before. At least, not that I'm aware of. There's should be a way to init our component knowing that it won't be unmounted. I know that back button can trigger that, but it's not a problem if we made the decision to do something in the init... right? There's some action that don't interact with the user that need to be done. I mean, React can be used for simple CSR SPA that don't need complicated stuff... but if I can't init my component easily, why would I use React? Having to rely on a third party library to fetch data cause it gets trigger 2 times... wait what?

[gaearon]

@prince-jn

The key thing here is that we don't have a clear easy way to do things like before.

Remove Strict Mode from your app, and it'll work as before.

Having to rely on a third party library to fetch data cause it gets trigger 2 times... wait what?

You don't need to rely on a third party library. There is literally no harm in doing data fetching twice in development. Every time you save a file, it also triggers a data fetch — so this isn't a new problem per se. This just makes it happen earlier. If your effects implement a cleanup function properly, then there is no actual problem with the behavior.

Please post a specific pattern you're having trouble with (and what kind of trouble you're seeing), and I can try to offer suggestions.

[prince-jn]

@gaearon Thanks for your answer.

Strict mode is activated by default and should be the future of React right? If not, why it's there by default? Why adding this behaviour? I don't want to go against the philosophy of React.

Of course, if I change something in my code... components must reload. I don't have any problem with that.

I think it's subjective to say that there's no harm in fetching data twice (dev). First thing I do after I write some code is to verify if my things work as intended (having performance in mind). How can I be sure everything is working well if the behaviour is different than production. You're telling me to switch in prod mode every time I write something. If one thing is different (dev vs prod), I will always think this my head... "maybe what I'm doing is wrong". Anyway, that the first time in 15 years of programming that I'm facing that kind of issue. It's counter intuitive.

[gaearon]

Strict mode is activated by default and should be the future of React right? If not, why it's there by default? Why adding this behaviour? I don't want to go against the philosophy of React.

It's technically not on by default when you use React (you have <StrictMode> somewhere) but yes, we'd like new apps to turn it on by default. That's why it's on by default in templates like CodeSandbox, Next.js, CRA, etc. You're right it aligns closer to the "future" but we don't want to make it too disruptive for code written assuming the old behavior. However, I don't currently think we'll ever force it to be on. I think of it more as a choice — do you want to "stress test" your components during development, or do you prefer to find bugs in production. We think stress testing is the right default for development, but we realize some people don't feel that way and would like to opt out.

How can I be sure everything is working well if the behaviour is different than production. You're telling me to switch in prod mode every time I write something

I don't think this is what I'm suggesting. On the contrary, I'm suggesting that, in our experience, what happens is the opposite: this check lets you find edge case bugs immediately that you would otherwise only discover in production or much later in development. I know this is counter-intuitive, but it is true. I can give a few examples if this doesn't sound realistic or doesn't align with your experience. One analogy is running a load stress test on your server. This is a bit similar, but for client code, and for different kind of bugs.

Anyway, that the first time in 15 years of programming that I'm facing that kind of issue. It's counter intuitive.

It is definitely counter-intuitive. Fwiw early on I was pretty strongly against this behavior. The more I've seen the kind of issues it uncovers in practice, the more I got "converted". :) I think with time you might feel the same way.

[prince-jn]

@gaearon Everyone use "create-react-app" when starting a new project from scratch (at least, me). It's way to complicated to do it alone. That's why I'm saying it's on by default.

I searched for hours how to fetch data on page load and I found this solution (it's like, every article/question older than 2 years was not good cause the React changed too much). It's depends which libs you're using and it's a mess looking for a solution. I found "useEffect" and it was exactly what I was looking for... and then I found that weird behaviour. I found last night "strict mode" was my problem after spending 1 hour searching why my component gets refreshed. First thing that comes in my mind: wow, that's a huge bug in React. Oh wow... finally, they want that behaviour? I thought I knew how react worked.

Maybe you can catch some bug with this "paradigm"... not sure it's the right word BUT... you can also cause new bugs having a different behaviour than production. Maybe it's not possible with this exactly behaviour but... you are in control of the source code. As a React user, I don't know if what I'm doing will work in 2 years. It's like a dilemma.

I will always be against that behaviour. You have other solution to tell users to tests their things... like a flag on <React.StrictMode testUnmountedOnDevEnv="true">.

[gaearon]

Maybe you can catch some bug with this "paradigm"...

You definitely can catch a lot of bugs this way. In fact, most classical examples with "fetch in useEffect" suffer from race conditions: https://maxrozen.com/race-conditions-fetching-data-react-with-useeffect. To avoid race conditions, you need to implement a cleanup function. And that's exactly the same solution that would prevent "refreshing" you are seeing in your component with Strict Mode. So in this case Strict Mode worked exactly as intended: it highlighted a potential bug in your code. See here for how to fix it: https://beta.reactjs.org/learn/synchronizing-with-effects#fetching-data

As for the lack of a unified data fetching approach, I hear you very well. We are adding something soon that will be the recommended approach and would remove the need to use effects for fetching.

[douglasjunior]

I understand the goal of this behavior, but it's many questions that a can't find an answer high now.

Example: How to handle animations that have to start just once and stops on unmount?

I believe that this kind of behavior shouldn't be affected by the "reusable state".

How to really handle things that MUST BE run just ONCE on first mount, e run just ONCE on last unmount?

[douglasjunior]

Another case, using the example from https://beta.reactjs.org/learn/synchronizing-with-effects#controlling-non-react-widgets

When "reusable state" arrives, will we have the modal opening and closing whenever the component is "remounted" temporarily?

[KianBadie]

@gaearon

You don't need to rely on a third party library. There is literally no harm in doing data fetching twice in development. Every time you save a file, it also triggers a data fetch — so this isn't a new problem per se. This just makes it happen earlier. If your effects implement a cleanup function properly, then there is no actual problem with the behavior.

I might have code smell elsewhere, but I think I have a situation where there is harm in fetching twice. On component mount I'm using the spotify api, which gives a access token based on a provided authorization code. The auth code only works once and will return errors on subsequent requests with the same code. How would you recommend to work around that? For now, I'm thinking of setting a state var to denote a failed request, but it feels awkward having a state where I have a valid token and a failed request to work with.

From what I've read before, a situation like this can actually happen in production, right? As in a component being mounted in quick succession and mounting multiple times quickly. If so, then I thank strict mode for helping me work with that in mind. If not, then I might need some help understanding why fetching data twice in strict mode would be unharmful in this situation.

[dtinth]

We just ran into this case where an email is being sent twice by this code:

useEffect(() => {
  sendVerificationEmail()
}, [])

…and an engineer fixed it by changing it like this, possibly following some tutorial:

const firstEffectRan = useRef(false)

useEffect(() => {
  if (firstEffectRan.current) {
    sendVerificationEmail()
  }
  return () => {
    firstEffectRan.current = true
  }
}, [])

It basically waits until the 2nd time the effect function is invoked before sending a verification email.

This resolved the problem in development mode but resulted in a completely broken feature in production mode. Luckily an automated UI test catches this issue. Hope this anti-pattern is documented somewhere.

[selsamman]

There are arguments for and against the current strict mode behaviour with respect to its faux renders and useEffects. While it might save careless programmers from failing to clean up after themselves it also causes pain. The pain includes unintuitive behavior that many initially believe is a bug, breaking the cardinal rule that development behavior should be as close as possible to production behavior and making certain patterns like the one in the original post near-impossible to implement. Searching these issues for "strictmode" one might think that the cure is as bad as the disease.

Consider how other code quality tools work. Lint, for example, detects patterns that might be bad and then gives you the opportunity correct them or opt-out by putting a comment in to ignore the check. In React it is all or nothing. If you are a component in a larger project or a library you can't opt-out selectively. Nor can you detect that strict mode is on so you can output a warning that your component will either not work or exhibit undesirable behaviour when StrictMode is on. Above all code quality tools don't affect the behaviour of your code making it work differently in production.

StrictMode in React is more akin to a "stress test". Would it not be safer to run it explicitly at some point during the development cycle rather than each and every time?

npm start --stress

One could even warn the user if stress had not been run prior to a production build

If the React team is adamant about keeping StrictMode in as default on CRA which effectively makes it default for the vast majority of users then please give people the ability to opt-out either through a wrapper function or something like the useEffectOnce that AgGrid came up with to overcome this behavior. I spent countless hours coming up with a similar solution for my Proxily library.

My two cents as a big fan of React.

[sonarpratikcodes]

We usually use 'useEffect' for making API calls. So while making an API call I faced the same issue that my API called twice while rendering. So I've resolved that problem using the below code:

useEffect(() => {
    if (isFetched) {
      const d = db.data;
      setData(d);
      console.log("db called");
    }

    return () => {
      setIsFetched(true);
    };
  }, [isFetched]);

I know I've manipulated the code but what so it is working very well... Have a nice day!!!

[Nmnjainsite]

@prince-jn

The key thing here is that we don't have a clear easy way to do things like before.

Remove Strict Mode from your app, and it'll work as before.

Having to rely on a third party library to fetch data cause it gets trigger 2 times... wait what?

You don't need to rely on a third party library. There is literally no harm in doing data fetching twice in development. Every time you save a file, it also triggers a data fetch — so this isn't a new problem per se. This just makes it happen earlier. If your effects implement a cleanup function properly, then there is no actual problem with the behavior.

Please post a specific pattern you're having trouble with (and what kind of trouble you're seeing), and I can try to offer suggestions.

const { currentUser } = useSelector((state) => state.auth);
const [userId, setUserId] = useState(null);
const uniqueId = currentUser?.uid;

useEffect(() => {
const fetchData = async () => {
try {
const res = await fetch(${baseURL}/api/users/github/${uniqueId});
const userData = await res.json();
setUserId(userData.map((el) => el.userId));
if (!userData || userData.length === 0) {
const createUserRes = await fetch(${baseURL}/api/createuser, {
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({
firstName: currentUser?.displayName.split(" ")[0],
lastName: currentUser?.displayName.split(" ")[1],
email: currentUser?.email,
website: "default",
role: "default",
billing: "default",
permission: "default",
googleId: "default",
githubId: uniqueId,
}),
});
const response = await createUserRes.json();
const userId = response.userId;
console.log(userId);
setUserId(userId);
} else {
console.log("User data already exists:", userData);
}
} catch (error) {
console.error("Error fetching user data:", error);
}
};

if (currentUser) {
  fetchData();
}

}, [currentUser]);

console.log(userId, "userId");

Here is my code: I want to fetch user data using the fetch API, and if the user doesn't exist, it will go into the 'else' block and create the user. The code works fine, but due to the useEffect hook being called twice, two users are being created, which is causing an issue. I have already removed strict mode and tried using useRef, but nothing seems to work for me.

[sonarpratikcodes]

@prince-jn

The key thing here is that we don't have a clear easy way to do things like before.

Remove Strict Mode from your app, and it'll work as before.

Having to rely on a third party library to fetch data cause it gets trigger 2 times... wait what?

You don't need to rely on a third party library. There is literally no harm in doing data fetching twice in development. Every time you save a file, it also triggers a data fetch — so this isn't a new problem per se. This just makes it happen earlier. If your effects implement a cleanup function properly, then there is no actual problem with the behavior.

Please post a specific pattern you're having trouble with (and what kind of trouble you're seeing), and I can try to offer suggestions.

const { currentUser } = useSelector((state) => state.auth);
const [userId, setUserId] = useState(null);
const uniqueId = currentUser?.uid;

useEffect(() => {
const fetchData = async () => {
try {
const res = await fetch(${baseURL}/api/users/github/${uniqueId});
const userData = await res.json();
setUserId(userData.map((el) => el.userId));
if (!userData || userData.length === 0) {
const createUserRes = await fetch(${baseURL}/api/createuser, {
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({
firstName: currentUser?.displayName.split(" ")[0],
lastName: currentUser?.displayName.split(" ")[1],
email: currentUser?.email,
website: "default",
role: "default",
billing: "default",
permission: "default",
googleId: "default",
githubId: uniqueId,
}),
});
const response = await createUserRes.json();
const userId = response.userId;
console.log(userId);
setUserId(userId);
} else {
console.log("User data already exists:", userData);
}
} catch (error) {
console.error("Error fetching user data:", error);
}
};

if (currentUser) {
  fetchData();
}

}, [currentUser]);

console.log(userId, "userId");

Here is my code: I want to fetch user data using the fetch API, and if the user doesn't exist, it will go into the 'else' block and create the user. The code works fine, but due to the useEffect hook being called twice, two users are being created, which is causing an issue. I have already removed strict mode and tried using useRef, but nothing seems to work for me.

[github-actions]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[github-actions]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!