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

Async storage lookup/default value issue #43

Closed
strarsis opened this issue Mar 13, 2022 · 20 comments
Closed

Async storage lookup/default value issue #43

strarsis opened this issue Mar 13, 2022 · 20 comments
Labels
bug Something isn't working

Comments

@strarsis
Copy link

@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
Copy link
Owner

I think your case will be fixed when 762fe4f lands in v16. I will write here when that happens so you can test it. How does that sound?

@strarsis
Copy link
Author

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

@strarsis
Copy link
Author

@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
Copy link
Owner

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

@strarsis
Copy link
Author

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

astoilkov added a commit that referenced this issue Mar 14, 2022
@astoilkov
Copy link
Owner

Done. Can you try out the new v16 release?

@strarsis
Copy link
Author

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

@astoilkov
Copy link
Owner

astoilkov commented Mar 14, 2022

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
Copy link
Author

strarsis commented Mar 14, 2022

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
Copy link
Owner

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
Copy link
Author

@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
Copy link
Owner

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
Copy link
Author

@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
Copy link
Owner

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?

@astoilkov astoilkov added the bug Something isn't working label Mar 16, 2022
@strarsis
Copy link
Author

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

@astoilkov
Copy link
Owner

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
Copy link
Author

strarsis commented Mar 17, 2022

@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
Copy link
Owner

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
Copy link
Author

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

@astoilkov
Copy link
Owner

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

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
Development

No branches or pull requests

2 participants