astoilkov / use-local-storage-state

React hook that persists data in localStorage
MIT License
1.17k stars 41 forks source link

React 18: value only is available on 2nd render cycle #56

Closed turingmachine closed 1 year ago

turingmachine commented 2 years ago

I believe this is by design, but can you elaborate why the value is only available on the 2nd render, which is triggered by use-local-storage-state itself?

import { useState, useRef } from 'react'
import useLocalStorageState from 'use-local-storage-state'

const App = ({ Component, pageProps }) => {
  const [triggerRender, setTriggerRender] = useState('a')

  const renderCounter = useRef(0)
  renderCounter.current = renderCounter.current + 1

  const [test, setTest] = useLocalStorageState('test', {
    defaultValue: 'DEFAULT',
  })

  console.log('cycle:' + renderCounter.current)
  console.log('test value:' + test)

  return (
      <a onClick={() => setTest('TEST')}>set Localstorage state</a>
  )
}

export default App
astoilkov commented 2 years ago

Hi, yeah, it's by design. There is no other way to make it work when ssr: true. Take a look at this comment where I explain this: https://github.com/astoilkov/use-local-storage-state/issues/54#issuecomment-1261800452.

I might add this to the documentation as it's creating a lot of confusion.

dherault commented 2 years ago

I'm having this issue with version 18 as well, using react 18 without StrictMode. Any ideas how to solve?

astoilkov commented 2 years ago

Hmm. You are right. I think I actually misread the issue. I will take a look at this in the next few days.

astoilkov commented 2 years ago

Hmm. Sorry. I was testing on Code Sandbox and there StrictMode is logging twice in the console (I didn't know that). I now tested this without StrictMode and can't replicate it.

Can you make a reproducible example?

Thanks!

astoilkov commented 2 years ago

@turingmachine Can you join in? Did you figure your issue out?

dalazx commented 2 years ago

I am observing the same issue which seems to be caused by the hydration process, but in my case there are more than 1 renders before the value is finally returned. The real issue is that it is impossible to tell whether the value is not ready or the value is not there at all. So as a workaround I came up with:

export function useIsLocalStorageReady() {
  const [isReady, setIsReady] = useLocalStorageState("__ready");
  useEffect(() => {
    if (isReady) return;
    setIsReady(true);
  }, [isReady, setIsReady]);
  return !!isReady;
}
const [value, setValue] = useLocalStorageState("value");
const isLocalStorageReady = useIsLocalStorageReady();
useEffect(() => {
  if (!isLocalStorageReady) return;
  // now value can be read
}, [isLocalStorageReady, value]);
astoilkov commented 2 years ago

@dalazx Hey, is this again with React 18?

Can you do a reproduction of this? This will help me a lot!

Thanks.

dalazx commented 2 years ago

@astoilkov here you go https://github.com/dalazx/use-local-storage-state-demo

hope this helps

image

overall I think the culprit is useSyncExternalState. I tried to debug it and saw that getSnapshot is not getting called during hydration.

astoilkov commented 2 years ago

You are right, the issue is useSyncExternalStore() and how it works internally. I didn't know that until now but TIL something. Here is the explanation.

If I change the code to this:

import {useSyncExternalStore} from "react";

function IndexPage() {
  const value = useSyncExternalStore(() => {
    return () => {}
  }, () => 'client value', () => 'server value')

  console.log(value)

  return (
    <>{value}</>
  );
}

export default IndexPage;

Where the important part is:

const value = useSyncExternalStore(() => {
  return () => {}
}, () => 'client value', () => 'server value')

console.log(value)

The console will log server value and then client value.

This seems like an internal React behavior. It first renders the server value and then the client value (only when it's different).

A more elegant solution to your problem would be:

function useIsServerRender() {
  const isServerRender = useSyncExternalStore(() => {
    return () => {}
  }, () => false, () => true)
  return isServerRender
}

I will update the documentation to clarify this behavior because it's really confusing.

pitkes22 commented 1 year ago

It would be really nice if isServerRender would be returned in extra data (in the object that now has isPersistent ...) since now it is not possible to distinguish if the value is defaultValue because of the first render, or if the value is not set.

astoilkov commented 1 year ago

Yep, I agree. I wanted to avoid that (I would have preferred one less value to return) but it seems necessary.


Can people in the discussion share why they needed to know from where the value comes? What will you do with the value?

I'm asking because it's also dangerous to provide that value because the user shouldn't change the rendered HTML (React doesn't allow that).

pitkes22 commented 1 year ago

My use case is that I need to run the side effect only when the user has no value set in localStorage.

So I have a useEffect where I check if the state is null (null is my default value), and if so I execute my side effect. Currently, it is not possible for me to achive this since:

  1. I get defaultValue during SSR
  2. I get defaultValue during the first render
  3. I get defaultValue during the second render (since there is no value in LS) or get the actual value during the second render

So for me there is no way to know if the value is defaultValue because there is no localStorage available or it is just empty.


I agree with you that it can be dangerous to provide this value to the user since the whole point of this two-pass rendering in strict mode is to achieve the same output between SSR and the first render.

So the output between SSR and the first render must always be the same as this.

  1. SSR: The value is defaultValue and isServerRender is TRUE
  2. 1st Render: The value is defaultValue and isServerRender is TRUE
  3. 2nd Render: The value is defaultValue or actual value and isServerRender is FALSE

But from what I can see when I use the hook that you suggested above useIsServerRender it actually works like this.


Alternatively, quite a nice solution can be to instead of providing a boolean value to the user with the value isServerRender the hook may accept two kinds of defaultValue

astoilkov commented 1 year ago

I decided not to add a specific property for this use case. I might be wrong but it feels like an edge case.

What I did instead added an explanation for the issue and how to fix it in the readme that also points to this issue.