streamich / react-use

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

useLocalStorage shouldn't include "| undefined" in its type if an initial value is provided #1483

Closed JoshuaKGoldberg closed 7 months ago

JoshuaKGoldberg commented 4 years ago

What is the current behavior?

type Value = { hello: boolean };
const [value] = useLocalStorage<Value>('someKey', { hello: false });

value is now type Value | undefined, even though we know there's a correct initialValue provided.

Steps to reproduce it and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have extra dependencies other than react-use. Paste the link to your JSFiddle or CodeSandbox example below: https://codesandbox.io/s/competent-mcnulty-o7nde?file=/src/App.tsx

What is the expected behavior?

value should be type Value.

A little about versions:

alisherk commented 4 years ago

Hi: I am here as part of HacktoberFest. Can I fix this issue

kamilmielnik commented 3 years ago

value is now type Value | undefined, even though we know there's a correct initialValue provided.

  1. It's initialValue - not defaultValue.
  2. I guess it's like this because there's remove function, which implies the possibility of undefined.

I think there should be no remove function - set(undefined) should act like remove currently does. Of course this line would have to be removed: https://github.com/streamich/react-use/blob/db07ab6/src/useLocalStorage.ts#L55 And then we could replace all T | undefined with T, and it'd solve the problem.

Svish commented 3 years ago

@JoshuaKGoldberg I fixed this issue in August, but still waiting for my PR #1438 to be accepted. 😕

@kamilmielnik I also considered letting set(undefined) do the same as the current remove function, but when I tried to do it, I think it was the the types that became kind of an issue. I feel having a separate remove function makes the intent clearer anyways though. That's how the underlaying storage works as well; you have separate setItem and removeItem.

JoeDuncko commented 3 years ago

Hi all! @react-hookz/web, the new library by one of react-use's former maintainers (background here and here) has this fixed. Check out the docs for useLocalStorageValue.

For those interested, there's an official migration guide for migrating from react-use to @react-hookz/web.

Hope this helps!

JoshuaKGoldberg commented 7 months ago

Closing out my old issues I no longer have context on. If anybody else is still seeing this, I'd encourage you to file a new issue with more information. Cheers!