astoilkov / use-local-storage-state

React hook that persists data in localStorage
MIT License
1.09k stars 39 forks source link

Async storage lookup/default value issue #43

Closed strarsis closed 2 years ago

strarsis commented 2 years ago

@astoilkov: A React app can have a query parameter (e.g. ?appId=123) which is persisted using this use-local-storage-state hook. A default value is also set (-1). When the state is -1, an error is thrown (no App ID is set, the app needs an App ID to function). The problem is that the use-local-storage-state is asynchronous: In the first render the value is still the default value (-1), in the 2nd render the value is the one retrieved from local storage. This means that in the first render the app already shows an error message, as the error will also stop further renders.

How can I get around this?

astoilkov commented 2 years ago

I think your case will be fixed when https://github.com/astoilkov/use-local-storage-state/commit/762fe4f413d5bd0805eab7e8e22bf1b0875d7218 lands in v16. I will write here when that happens so you can test it. How does that sound?

strarsis commented 2 years ago

@astoilkov: That seems to be exactly what I want! I install directly from master and try it out.

strarsis commented 2 years ago

@astoilkov: The app build currently fails - probably because the main source is TypeScript and has to be compiled:

Module parse failed: Unexpected token (2:12)
File was processed with these loaders:
 * ./node_modules/source-map-loader/dist/cjs.js
You may need an additional loader to handle the result of these loaders.
| import useLocalStorageState from './src/useLocalStorageState'
> import type { LocalStorageState } from './src/useLocalStorageState'
| 
| export type { LocalStorageState }
astoilkov commented 2 years ago

I will soon make a new release. I will write you then so you can try with it.

strarsis commented 2 years ago

@astoilkov: Ah, that would be great! I was just diving into the react-app-rewired-webpack-typescript-loader rabbithole. 🐱

astoilkov commented 2 years ago

Done. Can you try out the new v16 release?

strarsis commented 2 years ago

@astoilkov: Alright, I installed v16, but the default value is still used at first render. Am I missing something?

astoilkov commented 2 years ago

Can you share your code? Or at least a snippet of the important stuff so I can get a better understanding of what's happening.

Note that it will render twice but the code will call useEffect() and useLayoutEffect() callbacks once.

strarsis commented 2 years ago

Sure!

import {
    useCallback,
} from 'react'
import useLocalStorageState from 'use-local-storage-state';

const tableIdParamName = 'seat';
const localStorageSeatKey = 'seat';

export default function SeatProvider({ children }: { children: any }) {
    // persist (query is discarded by router)
    const [seatId, setSeatId, { removeItem: removeSeatId, isPersistent: isSeatIdPersistent }] =
        useLocalStorageState<number>(localStorageSeatKey, {
            ssr: true,
            defaultValue: -1, // -1 = no seat set
        });

    // initial seatId - use when set
    const url: URL = new URL(window.location.href);
    const params: URLSearchParams = url.searchParams;
    const initialSeatIdVal = params.get(tableIdParamName);
    if (initialSeatIdVal !== null) {
        const initialSeatId = parseInt(initialSeatIdVal);
        setSeatId(initialSeatId);
    }
    useCallback(() => {
        // This is called in 1st render, regardless of what is stored:
        if (seatId === -1) {
            throw new Error('No seat was assigned.');
        }
    }, [seatId]);

    // [...]
}
astoilkov commented 2 years ago

Hmm. I can't seem to replicate the issue. Here is the CodeSandbox I've experimented with: https://codesandbox.io/s/todos-example-use-local-storage-state-forked-s03chx?file=/src/App.tsx. Can you modify it in order to replicate the problem?

strarsis commented 2 years ago

@astoilkov: Thanks for your help! So I forked and modified your example a bit so the issue occurs (as in the app): https://codesandbox.io/s/todos-example-use-local-storage-state-forked-x1qf53?file=/src/App.tsx Am I using it wrong? When you check the console you see the default value -1 (I set the default value to -1 so I can mentally compare it with the app that has the same issue) and the error is thrown (hence no 2nd render with the loaded value).

astoilkov commented 2 years ago

You modified the example in a way where the defaultValue equals the actual value in localStorage-1. This behavior is expected and not a bug.

If you place localStorage.setItem('value', JSON.stringify(1)) after the import statements you will see that the error disappears — https://codesandbox.io/s/todos-example-use-local-storage-state-forked-0v3ti7.

strarsis commented 2 years ago

@astoilkov: This may explain the issue: I have a misunderstanding of how use-local-storage-state should be properly used. https://codesandbox.io/s/todos-example-use-local-storage-state-forked-e6e8qu?file=/src/App.tsx

  const [value, setValue] = useLocalStorageState("value", {
    ssr: true,
    defaultValue: -1
  });
  console.log(value);

  if (value === -1) {
    setValue(22);
  }

So instead of using setValue (the setter method in the array returned by useLocalStorageState(...), I have to use localStorage.setItem('value', [...]); instead? So setValue sets a value, but not persistently?

astoilkov commented 2 years ago

I should appologize as I didn't see the problem because it happens only on the first render after the key is changed. I've fixed the problem. Can you upgrade and let me know if it works?

strarsis commented 2 years ago

@astoilkov: The issue still persists 😿 . Default value in first render and set value in subsequent renders.

astoilkov commented 2 years ago

Ok. I think I know what is happening so I will chronologically explain what happened until now.

in version 15.0.0

In this version two bugs exist: 1) useEffect() and useLayoutEffect() are called twice when ssr is set to true 2) calling setValue() during render doesn't work (works only in useEffect() and useLayoutEffect())

in version 16.0.0

The 1. bug was fixed.

in version 16.0.1

The 2. bug was fixed.

Conclusion

The confusing part was that I thought your code will get fixed when I make the fixes (not sure if it did). However, you are right that there are two renders. Let me explain why.

When ssr is set to true there are two renders. If you set ssr to false there will be only one render (the hook works on the server even if ssr is false). The ssr property was introduced in version 15 and when ssr is set to true it handles hydration mismatches — you can read more about that here.

strarsis commented 2 years ago

@astoilkov: Thanks!

One question: When SSR is actually used and a hydration mismatch occurs (as the server side uses a different/initial value than the client in its storage), would this also result in a 2x rendering, with initial, then set value? Can the hydration mismatch be completely avoided when using SSR? I am just asking because I wonder how apps that use SSR can handle this problem.

astoilkov commented 2 years ago

You can read more here: https://reactjs.org/docs/react-dom.html#hydrate.

Conclusion is — you can avoid it by setting the suppressHydrationWarning but it's not recommended. You can read in the article why.

strarsis commented 2 years ago

@astoilkov: With ssr: false the issue indeed went away!

astoilkov commented 2 years ago

Cool. If that works for you and you don't have issues with hydration mismatches then this is a great solution.