Skip to content
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

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

Closed
JoshuaKGoldberg opened this issue Sep 1, 2020 · 5 comments

Comments

@JoshuaKGoldberg
Copy link

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
Copy link

alisherk commented Oct 2, 2020

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

@kamilmielnik
Copy link

kamilmielnik commented Oct 22, 2020

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
Copy link
Contributor

Svish commented Feb 22, 2021

@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
Copy link

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
Copy link
Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants