Option not to write default values to storage

[noahm]

I was looking into using this to replace some custom storage access hooks in a somewhat large codebase and one issue showed up that was pretty surprising. This hook writes the given default value into storage if nothing is found for the given key. This is undesirable behavior from my perspective and I'd like to opt out of it. I'm working around it for the time being by using my own wrapper hook around this one like so:

function useStorage<T>(key: string, defaultValue: T | null = null) {
  const [value, setValue, clearValue] = useStorageState(key);
  return useMemo(() => {
    if (value === undefined) {
      return [defaultValue, setValue, clearValue];
    }
    return [value, setValue, clearValue];
  }, [defaultValue, value, setValue, clearValue]);
}

What do you think about adding an option I could pass to useStorageState to opt-out of having defaults written into localStorage and filling up lots of keys with unnecessary values?

[astoilkov]

Hey, this behavior is borrowed from my other other library I've been maintaining for the past 4 years — https://github.com/astoilkov/use-local-storage-state. use-local-storage-state was working initially without writing the default value to storage. astoilkov/use-local-storage-state#26 made me change the default behavior. After that I haven't had someone request an option or change in behavior for that.

I do see rational in both cases so it feels like an option would make sense. I have a few questions:

• In your codebase, you probably have a lot of defaults the user might never change, right?
• What do you think is the main benefit of not writing? The saved space for the user, the higher chance localStorage MB limit won't be exceeded, or something else?
• Do you have ideas on how we can call this option?
• Is there are a case where you won't enable the proposed option? Considering just removing the behavior all together because it would make the package more lightweight.

Thanks in advance! If you don't have the time, I understand, I can also figure it out on my own.

BTW, are you trying to integrate use-storage-state in Twitch? This would be awesome!

[noahm]

Yes, this would be for use in the main Twitch website. We do have a lot of defaults the user would never change, and a lot of things that get added for a promotional event, then removed later.

What do you think is the main benefit of not writing? The saved space for the user, the higher chance localStorage MB limit won't be exceeded, or something else?

I think the main benefit is the ability to change the default in the future, and have that apply to users who haven't overwritten the default previously. Saving space for users is merely nice secondary benefit to me.

Do you have ideas on how we can call this option?

Something like persistDefault or writeDefault perhaps?

Is there are a case where you won't enable the proposed option?

Possibly, I can imagine a few possible scenarios, but I think it would be fairly unusual. I would have this option off by default for our primary storage hook. If you did remove it entirely, it's fairly easy to add ourselves I think?

function useStoragePersistDefault<T>(key: string, defaultValue: T) {
  useEffect(() => {
    // note: simplified, ignores serialization, etc
    if (localStorage.getItem(key) === null) localStorage.setItem(key, defaultValue)
  }, []);
  return useStorageState(key, { defaultValue });
}

I may have time to create a PR for this later this week, tbd.

Side note, if we do end up adopting this library, we may also want to extend it further to provide full same-tab reactivity for storage events, much like was discussed in astoilkov/use-local-storage-state#33 (comment) (but I would probably follow an implementation more like donavon/use-persisted-state)

[astoilkov]

Done. I've implemented a new storeDefault option — b145e6c.

If you like the implementation and the option name I can create a new release.

full same-tab reactivity

Exploring the code of use-persisted-state, I think use-storage-state has the same time of reactivity — multiple hooks for the same key on the page update when you call setState().

What I believe was discussed in astoilkov/use-local-storage-state#33 (comment) was the overwriting of localStorage so calling localStorage.setItem() will also update all hooks with the same key — https://github.com/tnrich/use-local-storage-state/blob/dca85f5337dfec889acd9cc9bca8746f31b461b1/src/useLocalStorageState.ts#L35-L74.

[noahm]

Thank you for such a quick response! The changes look great to me; looking forward to adopting it in a new release.

Re: same-tab reactivity

I see, thanks for clarifying. We do have a lot of places in our codebase that do write directly to storage, but everything goes through a wrapping interface. I think we can probably just dispatch a custom event from our wrapper if we decide to pursue fully consistent storage synchronization. (low priority)

[astoilkov]

Done. There is a new 4.0 release that includes the new storeDefault option.

I see, thanks for clarifying. We do have a lot of places in our codebase that do write directly to storage, but everything goes through a wrapping interface. I think we can probably just dispatch a custom event from our wrapper if we decide to pursue fully consistent storage synchronization.

Got it. You can also fire a storage event and use-storage-state will detect that and update.

Closing this issue. Thanks for your ideas, feedback, and help!

[noahm]

Thanks for being so responsive! I've updated my internal PR to the new 4.0 release and I'll let you know when we end up shipping it.

[noahm]

Just circling back to update that my internal PR merged and deployed last week, and this package is now in use broadly across Twitch web properties.

[astoilkov]

Wow, amazing. Is it ok to include this in the readme?

Thanks for giving this package a chance!