Default value ignores/overwrites previous localStorage state

[tnrich]

Hey there,

I am confused by the behavior of the defaultValue prop. I would imagine that say for the key of 'color', setting a defaultValue of 'red', would only set the attribute to 'red' if no other value was set for color already (in localStorage in my case).

Here's an example of what I find confusing:

https://codesandbox.io/s/purple-morning-1z2or?file=/src/App.js

This is confusing behavior IMO. I don't think the pre-existing localStorage keys should be clobbered by something that is named a defaultValue.

Thoughts @astoilkov ?

[astoilkov]

Wow. That's actually a very bad bug. I fixed it and published a new release.

Thanks, @tnrich!

Closing this issue.

[tnrich]

Awesome thanks @astoilkov !!

[tnrich]

@astoilkov just had time to test it and v13.0.0 doesn't appear to actually fix it: https://codesandbox.io/s/dreamy-cori-yh6y5

Still seeing the same issue. Not sure why the fix didn't take but you'll need to look at this again.

Thanks!

[astoilkov]

I see another problem, not the same one. Also, clicking the button does nothing?

[astoilkov]

To explain further. I know see how it works as expected but you will need to call JSON.stringify() on the colors in order to see them populated in the library. I can improve on that. I will think about how.

Can you explain what problem you see when you click the button?

[tnrich]

@astoilkov sorry I must have made a mistake. I could have sworn v13 had the same problem as v12 but after looking again, it does seem v13 is working (sort of). On v12 pressing the button causes the text to be updated.

Is the other problem you're referring to is that the color and name are still getting displayed incorrectly?

They're still not respecting what was already set in localStorage.

Thanks!

[astoilkov]

sorry I must have made a mistake. I could have sworn v13 had the same problem as v12

No problem. It may have been caused by some Code Sandbox caching issue and you could have been running the older version without knowing.

Is the other problem you're referring to is that the color and name are still getting displayed incorrectly?

Yes. Let me explain why this happens. When use-local-storage-state is storing and retrieving something into the state it uses JSON.stringify() and JSON.parse() to retrieve it. When a string is stored, it is stored with quotes "red". In your case the value is without quotes and JSON.parse() fails which causes use-local-storage-state to use the default value.

I can fix the issue by making an exception for strings but I'm not sure if it's a good idea. What do you think?

[tnrich]

@astoilkov ok I see.. I played around more with the codesandbox and was able to reproduce what you described above: https://codesandbox.io/s/headless-sun-rkjz3?file=/src/App.js

I personally think that this is a gotcha and not what I would have been expecting. I'm not sure what the best solution is but it definitely feels funny to me that the localStorage value is being stored as a string with quotes around it. At least for my use case almost all (if not all) of the values I'm storing using use-local-storage-state are either booleans or strings.

What are the downsides to making an exception for strings so that they can be stored "naturally" in localStorage?

Thanks for explaining!

[astoilkov]

What are the downsides to making an exception for strings so that they can be stored "naturally" in localStorage?

I can think of one problem. If you save the string true or 3 then it will be treated as a string but you could have meant it to be boolean or number because JSON.stringify() produces the same result. I can make it a fallback — if JSON.parse() throws an error and fails then interpret it as a string.

[tnrich]

@astoilkov Hmm.. I can't say I like either option much. Simple values like booleans and numbers should be supported still. Also assuming a string on a JSON.parse() fallback still means that if no value were set in localStorage, then use-local-storage-state would add its defaultValue as a string wrapped in quotes (at least I think that is the implication).

Ideally a solution would support booleans/numbers/strings as well as stringified JSON without strings getting the extra wrapping quotes.

Might another option be to add an option to do something like this:

useLocalStorageState('todos', ['buy milk']) //get's stringified like normal
useLocalStorageState('color', 'red', {noParse: true}) //add a new options argument to allow simple values to be treated differently (wouldn't get JSON stringified/parsed) 

same format for createLocalStorageStateHook

const useTodos = createLocalStorageStateHook('todos', [
    'buy milk',
    'do 50 push-ups'
]) //get's stringified like normal
const useToggledOn = createLocalStorageStateHook('isToggledOn', true, {noParse: true}})  //doesn't get stringified/parsed
const useColor = createLocalStorageStateHook('isToggledOn', 'blue', {noParse: true}})  //doesn't get stringified/parsed

Obviously the option name could be changed.. noParse or simpleValue or isSimple or isRaw are a few names that come to mind for me.

Thoughts?

Thanks!

[astoilkov]

I can't say I like either option much.

I agree. I also don't like either option.

Might another option be to add an option to do something like this

I don't like the option either. Let me explain why:

• I think that an option is not the right balance between complexity in the library and what the developer can handle on its own. For example, a user of the library can do one of three things:
  • Use JSON.parse() and JSON.stringify() to store your values
  • Import the storage object from use-local-storage/storage and use it instead
  • Use store2 when storing and retrieving values
• The solution won't be perfect. What if you wanted to put the string true in localStorage but use-local-storage-state assumes you wanted to put a boolean true. Even if I implement a solution the solution would never be able to guess the right type and it might do something unexpected for the user.
• I was looking at what store2 parsing code logic does and it tries to parse and if it can't it returns just the raw value. However, it doesn't have an option like { noParse: true }. That's the most popular library dealing with localStorage and they haven't decided to add something that handles such cases. This makes me think that it might not be the right move.

Conclusion

I think the best solution and also something actionable is to add an explanation in the readme on how to handle the data stored by use-local-storage-state outside of React hooks.

[tnrich]

Hey @astoilkov I really wanted the strings to "just work". I also was saddened that other changes to localStorage (in the same tab) wouldn't be registered by use-local-storage-state. Because of that I decided to fork this repo and add some custom fixes - https://github.com/tnrich/use-local-storage-state

You can play with the differences between the two libraries here: https://codesandbox.io/s/todos-example-use-local-storage-state-forked-uppgko?file=/src/App.tsx

[astoilkov]

Looks good. It's great to see people extending this library to make it satisfy their needs and use cases.

[rsodre]

I had this same problem, solved by bypassing the serializer:

import { useMemo } from 'react'
import useLocalStorageState from 'use-local-storage-state'

const _noSerializer = {
  stringify: (s) => (s),
  parse: (s) => (s),
}

const useLocalStorageValue = (key, serialize = true) => {
  const options = useMemo(() => ({ serializer: serialize ? JSON : _noSerializer }), [serialize])
  const [value, setValue] = useLocalStorageState(key, options)
  return {
    [key]: value,
  }
}

export {
  useLocalStorageValue
}

[astoilkov]

For others, bypassing the serializer (like in the previous comment) is a great way to handle strings as discussed in this thread. This wasn't originally discussed as option because back then the serializer option didn't exist.