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

useSessionStorage returns undefined if the value is already set as a string (+ some suggestions) #335

Closed
jonlinkens opened this issue Jun 10, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@jonlinkens
Copy link

jonlinkens commented Jun 10, 2023

useSessionStorage will return undefined if a string value already exists in session storage under the same key. This is because the hook always parses state as a JSON object with JSON.parse, which will error on strings and thus return undefined.

This can be avoided by using JSON.stringify on the string you initially set in session storage, but this feels a little too opinionated and cumbersome to do everywhere you have this case.

This would be useful for places in code where hooks can't be used to initially set a value in session storage - i.e. at app initialisation, and then accessing/setting it via the hook elsewhere.

Steps to reproduce

https://codesandbox.io/s/loving-taussig-m3qtkm?file=/src/App.tsx

As in the sandbox, set a string value in session storage beforehand, then try and access it via the hook. You'll see a parsing error on ... error gets printed to the console.

This behaviour can also be seen with this test case failing:

test('existing string state is in the returned state', () => {
  window.sessionStorage.setItem('key', 'value')

  const { result } = renderHook(() => useSessionStorage('key', 'value'))

  expect(result.current[0]).toBe('value')
})

giving the output

 Expected: "value"
 Received: undefined

Further comments

As well as the above, initialValue is never set in session storage - which feels like expected behaviour to come from this hook.

Additionally, there is no equivalent to useReadLocalStorage where one can pass through only a session storage key and get the value (or return undefined if doesn't exist) like so:

const value = useReadSessionStorage('test-key')

Proposed changes

  • To fix the existing string returning as undefined bug, I see two options:
    • Use JSON.stringify on the item value inside the hook, although I think this might introduce some issues
    • Become less opinionated, by adding some an optional options object argument:
      • parseAsJSON: boolean - default to true to avoid breaking changes
      • parser: (value: string | null) => T | undefined - allow custom parsers to be used in place of the existing parseJSON inside the hook
      • An accompanying serializer function
  • Set the initialValue in session storage, if one doesn't already exist
  • Add a useReadSessionStorage hook with similar behaviour to useReadLocalStorage
  • initialValue is a bit of a confusing name - right now it acts more as a defaultValue, so could rename to that (although with the setItem change above this would be fine as is)

I'd be more than happy to pick this up in a PR, can hopefully submit some time soon.

@simbesh
Copy link

simbesh commented Jun 23, 2023

Just ran into this same bug, my hack workaround is to just return the value if initialValue is a string or undefined:

useSessionStorage.ts

if (item) {
    if (typeof initialValue === 'string' || typeof initialValue === 'undefined') {
        return item as T
    }
    return (parseJSON(item) as T)
}
return initialValue

@AviDuda
Copy link

AviDuda commented Jun 27, 2023

The same thing happens in useLocalStorage and useReadLocalStorage.

@juliencrn
Copy link
Owner

Thank you @jonlinkens for the detailed bug report and the good proposals

#439 should fix most of the points here.

useReadSessionStorage should come asap too.

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
4 participants