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

useReadLocalStorage should not always parse value as JSON #240

Closed
kevinxh opened this issue Oct 18, 2022 · 3 comments
Closed

useReadLocalStorage should not always parse value as JSON #240

kevinxh opened this issue Oct 18, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@kevinxh
Copy link

kevinxh commented Oct 18, 2022

See https://github.com/juliencrn/usehooks-ts/blob/master/src/useReadLocalStorage/useReadLocalStorage.ts#L18, there is an incorrect assumption where the localstorage value is always JSON, but this is not true, you can have a string.

Assume you have the following item in your localstorage

key value
customer_id abcdefg
// this returns null
useReadLocalStorage('customer_id')

Screen Shot 2022-10-17 at 10 13 52 PM

proposal

Treat localstorage values as type string, not JSON. Users can always do Json.parse() in the user land.

@kokosin4ik
Copy link

Same problem for the useLocalStorage hook

@valyrie97
Copy link

useReadLocalStorage contains logic that links it to useLocalStorage (the custom event to update the hook state). The implication is that the prior is meant to be used in conjunction with the latter. Since useLocalStorage guarantees a value that is deserializable with JSON.parse, I think it stands to reason that it does that.

However, certainly, as this issue exists, there is a need for just interacting with the raw data. To me theres a couple of solutions on first glance that I think make sense:

  • Create raw variants of the mentioned hooks that dont use JSON at all. Possibly, since the values are not controlled by the hooks, the custom event should be removed as well, so as to not give false hope of values updating, when localstorage is used, say, by another library that doesnt fire the event. This could be remedied with the hook having two returns: value and a refresh function.
  • Allow both hooks to take an option that defines the serialize and deserialize functions for the data. This would allow you to completely bypass x => x, keep using json, or create (and separate) your own custom logic for any given key. The data format may be controlled outside of your code, but we can facilitate a custom transform to make the retrieval operate how its meant to.

@juliencrn
Copy link
Owner

Thanks all 🙌

#439 should fix most of the points here.

Feel free to re-open the issue if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants