-
-
Notifications
You must be signed in to change notification settings - Fork 425
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] useLocalStorage's setValue should be callable in render phase (like a normal useState
)
#549
Comments
While I agree with the fact that it should be possible to set a value in the render phase, it's not a recommendable approach as the setter triggers a re-render every time the component is rendered. This can lead to performance issues and unexpected behavior, especially if you're updating state frequently or performing expensive operations. This instead would be the right approach IMHO: useEffect(() => {
if (lsCount === 0) {
setLsCount(1);
}
}, [lsCount]); |
@juliencrn one last attempt to convince you:
This can also happen with
I think it is actually the recommended approach in the official React documentation (cf. https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes). They recommend not using an effect and just setting the value in the render phase: "Start by deleting the Effect. Instead, adjust the state directly during rendering" |
@juliencrn I also just wanted to add that if you change your mind, I'm happy to create a PR with the proper test coverage for this. |
Oh, I didn't know about that, interesting. So, I'm open to considering a fix, thanks by advance! |
@juliencrn great. Leave it with me. I'll take a look at this over the week end. |
Any update on this matter? |
Describe the bug
Calling the setter of
useLocalStorage
in a render phase triggers the error with the messageCannot call an event handler while rendering.
. It is becauseuseEventCallback
is used internally, but I thinkuseCallback
should be used instead since it is not conceptually only an event handler.To Reproduce
See the reproduction sandbox, but the gist of it is:
Expected behavior
It should be possible to set a value in the render phase.
The text was updated successfully, but these errors were encountered: