streamich / react-use

React Hooks — 👍
http://streamich.github.io/react-use
The Unlicense
41.59k stars 3.14k forks source link

useLocalStorage incorrectly states the value could be null when providing a default value #2293

Open AaronLayton opened 2 years ago

AaronLayton commented 2 years ago

What is the current behavior?

When providing a default value to useLocalStorage it still says that the return value could be undefined

What is the expected behavior?

I would expect that if no localStorage key is present then the default value would be passed back, causing the value to never be undefined

JadeMugs commented 2 years ago

Hi, useLocalStorage accepts an initialValue and not a defaultValue, which means that it can be undefined at a later time. For example, if you call theremove callback, the value is removed from the local storage, and the hook state becomes undefined.

AaronLayton commented 2 years ago

If I pass in the initialValue for a new user visiting my site, the current setup means I would need a useEffect to pick up the change to undefined and then set the value to what ever I need the new users to have initially.

This feels like initialValue is the same as defaultValue in that if I pass in an initialValue and there is no localStorage key already set then it would probably be more useful to return the initialValue and then set the localStorage key to the initial value.

JadeMugs commented 2 years ago

So, you are removing the current storage when a user logs out with the remove function, right? What about using the set function with the initialValue as the argument instead?

I think that removing the null or undefined values is not the right solution because you may need to use these in the set function.

aaronyo commented 2 years ago

@JadeMugs TypeScript will still give you an error that the value could possibly be undefined.

I do something like this to avoid that typing error whenever I useLocalStorage:

  const [value, setValue] = useLocalStorage(
    'storage-key',
    initialValue,
  );
  assertIsDefined(value); //helper asserts val is NonNullable
  ...

It would be nice if useLocalStorage could be a drop in replacement for React.useState.

Is there a use case where you would want your component to be re-rendered after calling remove (and before re-initializing)? So far, I don't want undefined in the render loop. If I did, I could still be explicit about that by calling set(undefined).

Since I don't want to render undefined, I don't plan on calling remove. Instead, I'll clear local storage on logout in one, more global place.

shennan commented 1 year ago

I propose allowing a defaultValue in the options:

const [value, setValue] = useLocalStorage('myBool', false, { defaultValue: false })

This is a non-breaking change.

miriamhorlick commented 1 year ago

A workaround that worked for me :

const [value = false, setValue] = useLocalStorage('myBool', false)