useLocalStorage's setValue(value => ...) passes the initial value instead of the current value

[dantman]

What is the current behavior?

If you use the function form of useLocalStorage's setValue the callback receives the initial value instead of the current value.

Cause of the issue

This part of the code passes state to the callback. However this comes from the closure and because state is not part of the deps array the set function is created on initial render and a new callback is not created when state changes.

https://github.com/streamich/react-use/blob/edbb10580e40520cf0552e089e6929436028b8fb/src/useLocalStorage.ts#L53

Potential fixes are A) passing state to the deps array (which will have the side effect of recreating set on every state change, which is different than how setState works; B) maybe reorganize the callback implementation to start with a function form of setState and do the serializing and localStorage inside the setState callback.

What is the expected behavior?

setValue(value => ...) should pass the current value to value the same way that useState's setValue does.

[sirjuan]

C) using current state by utilizing useLatest hook.

[makker]

+1 for option C

[Svish]

Why +1 for useLatest? I have a PR #1438 which solves several issues, including this one, and found using the updater function (your option B?) work pretty well. Unnecessary to pull in another hook, I feel. 🤔

[makinde]

Are there any maintainers looking at any of the PR's? It would be great to get this fixed. It's a pretty glaring bug that's been sitting around for many months.

[sirjuan]

@Svish

Yes, looks like your updater function solution would work pretty well, but it relies on user to memoize options object in order to keep set stable (like React.useStates setter is). I think it should work the same way. For that I would use refs for serializer and deserializer, either with useLatest or similar functionality without importing.

Maybe even using ref for set itself, since I don't think it should break referential stability even if key changes.

[Svish]

@Svish

Yes, looks like your updater function solution would work pretty well, but it relies on user to memoize options object in order to keep set stable (like React.useStates setter is). I think it should work the same way. For that I would use refs for serializer and deserializer, either with useLatest or similar functionality without importing.

Maybe even using ref for set itself, since I don't think it should break referential stability even if key changes.

Might look into that and update my PR, if anyone looks at it eventually.

[sirjuan]

Might look into that and update my PR, if anyone looks at it eventually.

I think I might close my PR (#1506 ) in favor of yours, since it has more potential, if it helps even a little bit.

[ianstormtaylor]

Just lost so much time to this... really bad that it pretends to mimic the useState API but passes in a non-changing value to the callback function.

[lmossman]

@streamich @AlexandreT-DevId @willnguyen1312 @suisous are any of you able to look into a fix here? This PR (https://github.com/streamich/react-use/pull/1438) solves this glaring bug in addition to other problems I've also ran into, and it would be great to get that in.