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

Bug: useMemo hook executes twice #24935

Closed
dmytro-vasylenko opened this issue Jul 15, 2022 · 20 comments
Closed

Bug: useMemo hook executes twice #24935

dmytro-vasylenko opened this issue Jul 15, 2022 · 20 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@dmytro-vasylenko
Copy link

dmytro-vasylenko commented Jul 15, 2022

I'm using useMemo hook with an empty dependency array in a component withlazy + Suspense, so I expect the function inside useMemo will be called once, but sometimes the function is called twice.

No StrictMode, no rerenders.

useMemo(() => {
  console.log('useMemo');
}, []);

React version: 18.2.0

I can't reproduce it with version 17.0.2

Steps To Reproduce

Please, take a look at the simplified example. I could reproduce it on a regular basis after I've added setState call inside useMemo. As the issue is hard to reproduce, there is a script that reloads the page until the bug appears.

Link to code example: https://codesandbox.io/s/smoosh-forest-g6ft5o

Pay attention, that function in useEffect was called once, which is expected behavior, but useMemo was called twice.

In the real project, there is no setState call inside useMemo and no warnings, but anyway I meet the issue every 10-20 page reloads.

If I delete lazy it works as expected. If I drop LongComponent it works as expected.

The current behavior

The function passed to useMemo is executed twice despite the empty dependency array, and the component wasn't rerendered.

console:
  useMemo
  useMemo
  useEffect

The expected behavior

The function passed to useMemo is executed only once.

console:
  useMemo
  useEffect
@dmytro-vasylenko dmytro-vasylenko added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jul 15, 2022
@dmytro-vasylenko dmytro-vasylenko changed the title Bug: useMemo hook executed twice Bug: useMemo hook executes twice Jul 15, 2022
@gaearon
Copy link
Collaborator

gaearon commented Jul 15, 2022

Why does it matter if it executes once or twice? Since any function inside useMemo should be pure, it should make no difference. Is this a performance concern, or does it change the behavior in some observable way (when you follow the rules)?

@TheDevCactus
Copy link

TheDevCactus commented Jul 15, 2022

After playing with the sample you provided the issue is that the setTimeout you are initiating in the useMemo executes before the component has a chance to render. If you increase the timeout on that setTimeout you'll notice the error go away, Or you can swap out the LongComponent for just like a <h1>hi mom</h1>, everything will function as normal.

In the sample you provided the error
memo-error
This pretty much says the exact problem. Since the setTimeout runs before the initial render your setState calls from within it causes this bunk behavior.

Honestly the contents of the useMemo really feel like the kind of thing I'd put in a useEffect. Especially since there isn't anything being returned from it

@dmytro-vasylenko
Copy link
Author

@TheDevCactus I understand that there is a warning, but this is not an error. The example is just a simulation how to make the bug to appear. I hope it will help somehow. React doesn't show any warning or something in my project, so it was hard enough to spot the issue.

@dmytro-vasylenko
Copy link
Author

@gaearon Yeah, it causes unpleasant problems with performance, because the returned value from useMemo is an object. I can fix that, but still this behavior seems to me strange and unexpected.

@qiqo
Copy link

qiqo commented Jul 18, 2022

Why does it matter if it executes once or twice? Since any function inside useMemo should be pure, it should make no difference. Is this a performance concern, or does it change the behavior in some observable way (when you follow the rules)?

just to add that I'm also having this issue now with the same version and found this thread, this is a problem because I use the hook to control debounce to an expensive REST api, and if it is executed twice (which it is now) then that's a problem because the target API is used twice than it should which can cause race conditions transactional issues and at the production cloud level that amounts to twice the consumption on cloud function/serverless apis which eats up half of my paid for allocation.

@832bb9
Copy link

832bb9 commented Jul 21, 2022

this behavior seems to me strange and unexpected

The docs explicitly says:

'You may rely on useMemo as a performance optimization, not as a semantic guarantee.'.

You can try achieve semantic guarantee using https://reactjs.org/docs/hooks-faq.html#how-do-i-implement-getderivedstatefromprops.

@uditalias
Copy link

Same here, I'm using a dependency injection framework factory method inside useMemo to create one instance of a model inside my component, using react 18 triggers useMemo few times for no reason.

@832bb9
Copy link

832bb9 commented Aug 31, 2022

Same here, I'm using a dependency injection framework factory method inside useMemo to create one instance of a model inside my component, using react 18 triggers useMemo few times for no reason.

Please check this https://reactjs.org/docs/hooks-faq.html#how-to-create-expensive-objects-lazily. You can manually save this instance to ref on very first render (or even lazily).

@uditalias
Copy link

Same here, I'm using a dependency injection framework factory method inside useMemo to create one instance of a model inside my component, using react 18 triggers useMemo few times for no reason.

Please check this https://reactjs.org/docs/hooks-faq.html#how-to-create-expensive-objects-lazily. You can manually save this instance to ref on very first render (or even lazily).

This is gold! thank you!

@rickhanlonii
Copy link
Member

I understand that there is a warning, but this is not an error.

@dmytro-vasylenko the message says "warning" but the log level is error and we consider this an error. It's hard to say exactly what's going on here because it's violating a few patterns, so it's not unexpected that we end up needed to re-memoize the value. Are you able to create a repro that's closer to your use case that doesn't error or depend on calling setState inside useMemo to reproduce?

@SpadarShut
Copy link

I call URL.createObjectURL and need to revokeObjectURL on unmount.
What would be the way to make sure I can revoke every created blob URl?

@832bb9
Copy link

832bb9 commented Sep 23, 2022

I call URL.createObjectURL and need to revokeObjectURL on unmount.

What would be the way to make sure I can revoke every created blob URl?

Seems like useEffect is what you need.

@kolserdav
Copy link

kolserdav commented Sep 30, 2022

This is a real useMemo in React 18.2.0 bug. Here is a simple example where useMemo is triggered twice for each dependency value.

https://codesandbox.io/s/compassionate-swartz-wd3j94?file=/src/App.js

https://iili.io/LEGLkg.png

Or is this normal in strict mode?

@kolserdav
Copy link

This is a real useMemo in React 18.2.0 bug. Here is a simple example where useMemo is triggered twice for each dependency value.

https://codesandbox.io/s/compassionate-swartz-wd3j94?file=/src/App.js

https://iili.io/LEGLkg.png

Or is this normal in strict mode?

Yes, I see this is normal https://reactjs.org/docs/strict-mode.html

@mhtamun
Copy link

mhtamun commented Jul 23, 2023

Why does it matter if it executes once or twice? Since any function inside useMemo should be pure, it should make no difference. Is this a performance concern, or does it change the behavior in some observable way (when you follow the rules)?

just to add that I'm also having this issue now with the same version and found this thread, this is a problem because I use the hook to control debounce to an expensive REST api, and if it is executed twice (which it is now) then that's a problem because the target API is used twice than it should which can cause race conditions transactional issues and at the production cloud level that amounts to twice the consumption on cloud function/serverless apis which eats up half of my paid for allocation.

I am facing exactly same issue!

@pauldraper
Copy link

I call URL.createObjectURL and need to revokeObjectURL on unmount.

This is exactly how I ran into this!

Seems like useEffect is what you need.

useEffect would involve an unnecessary re-render. There's no reason not to be synchronous.

FWIW,

function useObjectUrl(blob) {
  const ref = useRef();
  const dispose = () => ref.current && URL.removeObjectURL(ref.current.url);
  useEffect(() => dispose, []);
  if (ref.current && blob === ref.current.blob) {
    return ref.current.url;
  }
  dispose();
  const url = URL.createObjectURL(blob);
  ref.current = { blob, url };
  return url;
}

@theKashey
Copy link
Contributor

Recently we faced the issue with useMemo being called multiple times, and sometimes more than just 2, with useEffect being called once. Wondering how it's working underneath...

Investigation led us to the moment where React picks the right hook implementation -

if (current !== null && current.memoizedState !== null) {
ReactCurrentDispatcher.current = HooksDispatcherOnUpdateInDEV;
} else if (hookTypesDev !== null) {
// This dispatcher handles an edge case where a component is updating,
// but no stateful hooks have been used.
// We want to match the production code behavior (which will use HooksDispatcherOnMount),
// but with the extra DEV validation to ensure hooks ordering hasn't changed.
// This dispatcher does that.
ReactCurrentDispatcher.current = HooksDispatcherOnMountWithHookTypesInDEV;
} else {
ReactCurrentDispatcher.current = HooksDispatcherOnMountInDEV;
}
} else {
ReactCurrentDispatcher.current =
current === null || current.memoizedState === null
? HooksDispatcherOnMount
: HooksDispatcherOnUpdate;

In our case it was picking HooksDispatcherOnMount where useMemo callback has a direct execution and the only way to pick the branch is not to have memoizedState in the current fiber.

Meaning:

So the only "stable" way to perform operation once is to perform it in useEffect(hello StrictMode) and save in useState causing re-render, or placing components with useMemo a bit more strategically to have them being rendered a little more stable, ie "above Suspense boundaries".

Copy link

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 github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Apr 10, 2024
@kaifaty
Copy link

kaifaty commented Apr 17, 2024

useRef content clears too :(

@github-actions github-actions bot removed the Resolution: Stale Automatically closed due to inactivity label Apr 17, 2024
@kassens
Copy link
Member

kassens commented Apr 17, 2024

With Suspense, it's expected that some components re-execute when the suspense resolves (all the components up to the component that suspended). Generally, components need to be designed to be able to re-run or run without mounting, for example when something else rendered before the suspense was resolved.

@kassens kassens closed this as completed Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests