Description

The type of the returned value from useLocalStorage is currently typed as T | undefined, but when providing an initialValue, it should be unnecessary to check for undefined, since, if it is, you should get the initialValue of type T instead.

To solve this I have added two TypeScript overloads for useLocalStorage:

The case when you provide initialValue, value is T
The case where you don't, value is T | undefined.

Additionally:

I changed the type of the setter parameter type to not allow undefined, since that's a no-op anyways.
Changed the remove callback to set state back to initialValue instead of undefined.
Wrote a test for remove as I couldn't find any existing ones.

I have tested to install this package locally (via yarn pack) to check if the types look correct in a project using it, and as far as I can see, it does.

One thing I'm a bit uncertain of, is if the set callback should allow undefined, when initialValue is undefined. It would make sense, but since that's a no-op anyways, it kind of doesn't. So my conclusion is that set should never allow undefined, unless undefined actually does something. It could for example be a second way to remove/unset the value (i.e. do exactly the same as remove), but then you might as well just use remove. So, yeah...

Type of change

[x] Breaking change (fix or feature that would cause existing functionality to not work as before)

The typings of the useLocalStorage hook and callbacks have been improved.
This might cause some minor needs for adjustments in typings where the hook is used.
That remove now sets state back to initialValue rather than undefined.
This might cause need for adjustment if someone has provided an initialValue other than undefined, and relies on the value being set to undefined rather than returned to the provided initialValue after calling remove.
The useSessionStorage hook signature is now equal to useLocalStorage.
This means the last parameter should now be an options object instead of a boolean, but I've made it backwards-compatible so that true gives same result as { raw: true }

Checklist

[x] Read the Contributing Guide
[x] Perform a code self-review
[x] Comment the code, particularly in hard-to-understand areas
[x] Add documentation
[x] Add hook's story at Storybook
[x] Cover changes with tests
[x] Ensure the test suite passes (yarn test)
[x] Provide 100% tests coverage
[x] Make sure code lints (yarn lint). Fix it with yarn lint:fix in case of failure.
[x] Make sure types are fine (yarn lint:types).

Additionally...

After I initially created this PR, I decided to do some more minor fixes:

Moved my also not merged fix #1448 into this PR (stale state fix for updater function)
Added tests for previously untested serializer & deserializer options
Replaced useSessionStorage with a copy of useLocalStorage so they now have the same features and signature.

Aug 13 '20 13:08 Svish

Example of why the current types are annoying:

Here, there's no reason why value should be undefined, since I provide the hook with an initualValue that is guaranteed to be a boolean.

Aug 13 '20 13:08 Svish

I legit need this right now lol merge please! I'm having to sprinkle in a non-null assertion #ugly

Aug 28 '20 16:08 gongjoonamu

@gongjoonamu If you find yourself sprinkling many of those non-null assertions, if you haven't already done similar, here's a temporary "workaround" to keep the #ugliness is one place. So when this is released, you can just delete this, and "redirect" the imports back to the react-use one.

import { Dispatch, SetStateAction } from 'react';
import { useLocalStorage as useLS } from 'react-use';

type Return<T> = [T, Dispatch<SetStateAction<T>>, () => void];

// TODO: Remove when https://github.com/streamich/react-use/pull/1438 (hopefully) gets released
export default function useLocalStorage<T>(key: string, initialValue: T): Return<T> {
  return useLS<T>(key, initialValue) as Return<T>;
}

Aug 28 '20 17:08 Svish

Added another commit now, which replaces the current useSessionStorage with a copy of useLocalStorage (with storage replaced) because of the following:

It had the same typing issue as useLocalStorage
It did not have the same features, like custom serializer and such
It did not have any tests at all

Jan 05 '21 10:01 Svish

Would be nice if someone could review this soon. 🙏🚀

Jan 05 '21 10:01 Svish

Realized I had another PR with a fix related to useLocalStorage too (#1448), so decided to move that commit here and apply the fix for useSessionStorage as well.

I also fixed some eslint warnings, and added some tests for the serializer & deserializer options which were previously untested.

Jan 05 '21 12:01 Svish

Looks like this MR also will fix the return type of useSessionStorage?

Currently it is returning [T, (value: T) => void], and I am running into the problem that Typescript is complaining that I can't use the functional update for the setState function.

Whereas is should be, [T, Dispatch<SetStateAction<T>>], or since you use the copy of useLocalStorage [T, Dispatch<SetStateAction<T>>, () => void]

Jan 14 '21 14:01 sjiep

Looks like this MR also will fix the return type of useSessionStorage?

Currently it is returning [T, (value: T) => void], and I am running into the problem that Typescript is complaining that I can't use the functional update for the setState function.

Whereas is should be, [T, Dispatch<SetStateAction<T>>], or since you use the copy of useLocalStorage [T, Dispatch<SetStateAction<T>>, () => void]

Yeah, this PR makes them the same api and typings.

Jan 14 '21 14:01 Svish

Are there any issues with this PR?

I needed to switch a useLocalStorage using a custom (de)serializer with useSessionStorage and was disappointed to find that it was missing that functionality and I couldn't use it.

Feb 04 '21 05:02 dantman

Noticed some merge-conflicts had popped up here since last time, so I have rebased and fixed them all now.

Feb 22 '21 22:02 Svish

@streamich any chance you or another maintainer can look/merge?

Feb 23 '21 06:02 makinde

I closed my PR #1506 in favor of this one.

I would use useLatest or similar approach for serializer/``deserializeror evensetitself to maintain referential stability similar toReact.useState`.