Support false values with a true fallback value

[jaebradley]

Summary

This is a great utility hook, but I ran into some issues when storing booleans (specifically false with a true initial value).

Basically, when the local storage value is false (or more specifically, "false") and an initial value of true is specified, the value returned by the useLocalStorage hook is true.

There are certain UI cases where one would want the default behavior to be true but to still remember the case where the user has toggled that behavior to a false state (like remembering if a slideout is opened or closed).

I've gotten around this by using numerical values, but it would be great if this hook supported the case where the local storage value is false while the default behavior is true.

Reproducing the behavior

Here's a screenshot of the behavior

Here's a link to the underlying demo: https://stackblitz.com/edit/local-storage-false-example

Basically, toggle the button until the local storage value is false and then reload the page.

Discussion

I noticed that in a lot of places, the default value is only set / used if the parsed local storage value is falsy.

tryParse(localStorage.getItem(key)!) || initialValue

From the Storage#getItem MDN page it seems like the existence of a key is determined by whether null is returned by getItem.

Maybe it would be more appropriate to check if localStorage.getItem(key) is null before falling back to the initial value?

I know this is a breaking change, which would necessitate a v3.x.x release, but it might ultimately lead to more expected behavior?

[jharrilim]

Hi @jaebradley, thanks for your contribution! This should be appropriate to add as a patch as I'm sure the current behaviour is a bug.

[doytch]

@jharrilim, was the intention of the 2.3.0 release to include this bugfix? The date stamps appear to show it being released four hours after this PR was merged, but looking at the actual code that's pulled down from NPM, it's still using the incorrect pre-PR behaviour. E.g.,

[12:19:10] rehook2.3.0 : cat node_modules/\@rehooks/local-storage/package.json | grep "\"version\":"
  "version": "2.3.0"
[12:19:13] rehook2.3.0 : cat node_modules/\@rehooks/local-storage/lib/use-localstorage.js | grep -A 2 "function useLocalStorage"
function useLocalStorage(key, initialValue) {
    const [localState, updateLocalState] = react_1.useState(tryParse(localStorage.getItem(key)) || initialValue);
    const onLocalStorageChange = (event) => {

Could you release a 2.3.1 that includes this fix?

[jharrilim]

Hi @doytch, yes I believe the fix should've been there. Unfortunately I do not have publish rights to @rehooks on npm as I am not a part of this org. @iamsolankiamit may have to do it instead.

[doytch]

Ahhh OK, thanks for the clarification; I was just going off of the Github releases page 😆

[iamsolankiamit]

@doytch @jharrilim it is released under https://github.com/rehooks/local-storage/releases/tag/v2.4.0