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

Thoughts on missing localStorage? #5

Closed
rexxars opened this issue Apr 26, 2020 · 7 comments
Closed

Thoughts on missing localStorage? #5

rexxars opened this issue Apr 26, 2020 · 7 comments
Assignees
Labels
discussion Discussions on library API decisions

Comments

@rexxars
Copy link
Contributor

rexxars commented Apr 26, 2020

In certain contexts you may encounter browsers which have disabled localStorage.

If you're blocking cookies in Safari, for instance, simply trying to access localStorage (even for a typeof check) will throw an error.

In other cases/browsers, attempting to actually set an item will yield a quota exceeded error (they set the max quota size to 0 bytes).

An actual check for whether or not localStorage is "available and usable" would look something like:

const hasLocalStorage = (() => {
  try {
      if (typeof localStorage === 'undefined') {
          return false
      }

      // Can we actually use it? "quota exceeded" errors in Safari, etc
      const mod = '__lscheck'
      localStorage.setItem(mod, mod)
      localStorage.removeItem(mod)
  } catch (err) {
      return false
  }

  return true
})()

Even with such a check, the "solution" isn't obvious - what should a library such as this one do in this case? Throw? Warn? Pretend like it isn't happening?

Given that you often use this as a "drop-in" replacement for setState, I would at least expect it to fall back to using in-memory state - but in that case, how far do you go? Try to implement postMessage calls for cross-tab/window polyfilling of the missing storage events?

I'm raising this mostly to get your perspective, but I also feel that perhaps we should try to implement something that prevents this hook from potentially blocking the page from being rendered should the user have disabled localStorage.

@astoilkov astoilkov added the discussion Discussions on library API decisions label Apr 27, 2020
@astoilkov
Copy link
Owner

Respect for the type of issue you have created. This is what I call a way to approach problems – with questions.

These aren't easy questions. Can you give me a few days to think about it and we can discuss them after that?

@astoilkov
Copy link
Owner

I saw that type of thinking in your post: https://espen.codes/post/strictness-in-apis-or-why-hapi-rocks. Nice!

@astoilkov
Copy link
Owner

const [value, setValue, isPersistent] = useLocalStorageState('key')

What do you think about such an API? The hook will return an additional boolean to let you know if the data is currently being persisted in localStorage or not?

@rexxars
Copy link
Contributor Author

rexxars commented Apr 28, 2020

I like it! Leave it up to the app developer whether or not they care :)

astoilkov added a commit that referenced this issue Apr 29, 2020
After discussions:
- #5
- #2

I decided that the library should handle local storage throwing errors in some edge cases. The library shouldn't feel unstable and not work if you aren't using it correctly.
@astoilkov astoilkov self-assigned this Apr 29, 2020
@astoilkov
Copy link
Owner

I am done with the implementation. Can you check it out and tell me what you think?

@rexxars
Copy link
Contributor Author

rexxars commented May 3, 2020

This looks great!

@astoilkov
Copy link
Owner

Perfect. I am closing this issue then. If you have more valuable feedback please don't hesitate to write it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussions on library API decisions
Projects
None yet
Development

No branches or pull requests

2 participants