imbhargav5 / rooks

Essential React custom hooks ⚓ to super charge your components!
https://rooks.vercel.app
MIT License
3.19k stars 216 forks source link

"useLocalstorageState" implementation is confusing and probably harmful #1744

Closed gabrielhpugliese closed 6 months ago

gabrielhpugliese commented 6 months ago

If we type something like this:

const [value, setValue] = useLocalstorageState<string | null>('myKey');
  1. And we want to remove the key from the storage, we have to pass undefined to setValue as we can see in this line. But this can't work with TS because TS will stay that it only accepts string or null.
  2. If we try to set null to the value calling setValue(null) then the value will be "null" of type string but that's not what we wanted. We wanted the primitive null.
  3. Then we have the option of typing useLocalstorageState<string | undefined> which then by your implementation returns the primitive null which is also what we don't want.

And I don't want to type my value as string | null | undefined because it's way too broad.

gabrielhpugliese commented 6 months ago

Ok, that was a bad take because the localStorage only accepts strings as values