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

Can cause hydration mismatch with SSR #23

Closed
gragland opened this issue Feb 14, 2021 · 23 comments
Closed

Can cause hydration mismatch with SSR #23

gragland opened this issue Feb 14, 2021 · 23 comments

Comments

@gragland
Copy link

First off, really nice hook! One issue I'm seeing for SSR'ed apps is if the stored value affects DOM structure then there's going to be a hydration mismatch warning (for example Warning: Expected server HTML to contain a matching <div> in <div>).

One workaround:

export default function Todos() {
    const [query, setQuery] = useState('')
    const [todos, setTodos] = useLocalStorageState('todos', ['buy milk'])
 
    // Keep track of whether app has mounted (hydration is done)
    const [hasMounted, setHasMounted] = useState(false);
    useEffect(() => { 
        setHasMounted(true) 
    }, []);

    function onClick() {
        setTodos([...todos, query])
    }

    return (
        <>
            <input value={query} onChange={e => setQuery(e.target.value)} />
            <button onClick={onClick}>Create</button>
   
            {/* Only render todos after mount */}
            {hasMounted && todos.map(todo => (<div>{todo}</div>))}
        </>
    )
}

Maybe we can include a section in the readme that covers this? I'm not sure if there's an elegant way for the to hook to handle this. It could wait until mount to return the value from storage, but then that means an extra re-render for non-SSR apps. Thoughts?

More discussion:
https://twitter.com/gabe_ragland/status/1232055916033273858
https://www.joshwcomeau.com/react/the-perils-of-rehydration/

@astoilkov
Copy link
Owner

Hi. I started thinking about this topic after the discussion in one of the issues. You can read my thoughts there.

I've read the discussions in the links you provided – interesting content. I have one question that I'm confused about: Why the approach with hasMouted doesn't use useLayoutEffect()? Wouldn't this update faster and the user see less of a flicker (or maybe even no flicker)?

@gragland
Copy link
Author

gragland commented Feb 14, 2021

Why the approach with hasMouted doesn't use useLayoutEffect()? Wouldn't this update faster and the user see less of a flicker (or maybe even no flicker)?

Good point. That does avoid the flicker, but results in a warning due to useLayoutEffect being run the server [0]. I was able to work around that by using useEffect on the server instead. In case you're interested, this I'm now handling it in my code now.

// use-has-hydrated.js
import { useState, useEffect, useLayoutEffect } from "react";

function useHasHydrated(beforePaint = true) {
  const [hasHydrated, setHasHydrated] = useState(false);

  // To reduce flicker, we use `useLayoutEffect` so that we return value
  // before React has painted to the browser.
  // React currently throws a warning when using useLayoutEffect on the server so
  // we use useEffect on the server (no-op) and useLayoutEffect in the browser.
  const isServer = typeof window === "undefined";
  const useEffectFn = beforePaint && !isServer ? useLayoutEffect : useEffect;

  useEffectFn(() => {
    setHasHydrated(true);
  }, []);

  return hasHydrated;
}

// use-last-provider.js
const defaultValue = "facebook";
const useLastProviderBase = createLocalStorageStateHook("lastAuthProvider", defaultValue);

const useLastProvider = () => {
  const [lastProvider, setLastProvider] = useLastProviderBase();
  const hasHydrated = useHasHydrated();
  // We only return the stored provider value after the app has hydrated
  // so that it's undefined on both the server and (initially) the client.
  // This allows React to properly hydrate from server and the component
  // calling this hook doesn't have to worry about it.
  return [hasHydrated ? lastProvider : defaultValue, setLastProvider];
};

Do you think it would make sense to have an options arg for createLocalStorageStateHook with ability to specify that it's being used in an SSR environment and the value should be the defaultValue until after hydration? This would basically replace what I'm doing above.

const useLastProvider = createLocalStorageStateHook("lastAuthProvider", "facebook", { ssr: true });

[0] facebook/react#14927

@astoilkov
Copy link
Owner

I like the approach. I will experiment with it and see if I can add an additional hook to the library.

BTW have you tried using suppressHydrationWarning property described here? I am wondering if it's a better solution in general when using use-local-storage-state.

@gragland
Copy link
Author

gragland commented Feb 16, 2021

BTW have you tried using suppressHydrationWarning property described here? I am wondering if it's a better solution in general when using use-local-storage-state.

I didn't know about that, thanks! That seems to work fine for text content, but the issue is that there can be weird rendering bugs when hydration doesn't match, such as elements in the wrong place.

Also worth checking out the Material UI useMediaQuery hook. It does an extra render by default to solve this problem and has a noSsr option to disable. Here is the code.

@astoilkov
Copy link
Owner

@gragland I am now convinced that I should find a solution to this. I will think about it and let you know. Thanks for the entire discussion.

@astoilkov
Copy link
Owner

I found a solution I like. A hook called useSsrMismatch that is general purpose and will work for similar problems. Here is pseudo code:

const [todos, setTodos, isPersistent] = useSsrMismatch({
    beforeHydration: [defaultTodos, () => {}, true],
    afterHydration: useLocalStorageState('todos', defaultTodos)
})

@gragland What do you think?

There are some questions I still don't have answers to:

  • Do I open-source the useSsrMismatch() hook? Is it a common problem?
  • Do I integrate this into the library or just update the docs saying how to solve this problem?
  • What is the potential API for the SSR support?

@gragland
Copy link
Author

I really like the idea for useSsrMismatch and could see myself using it in custom hooks I write and when dealing with libraries that don't handle this. It seems like a much better solution than how I use useHasHydrated in my above example.

I think my preference would still be to have this integrated into the library and have it be opt-out like Material UI's useMediaQuery hook.

So maybe the API would look like:

// disable ssr support
useLocalStorageState('todos', defaultTodos, { noSsr: true });

// or
const useTodos = createLocalStorageStateHook('todos', defaultTodos, { noSsr: true });

The logic being that Next.js and Gatsby are growing in popularity and it's better that a non-SSR user have an unnecessary re-render than having SSR users run into subtle bugs or have to understand how to use useSsrMismatch. I'm assuming the performance cost of useLayoutEffect is pretty small, but probably worth looking into first.

@Dalez
Copy link

Dalez commented Jul 24, 2021

Did this get anywhere? I'm also running into the same problem.

I like the noSsr feature suggested above and I also agree that it would be good to include it in the library so that developers don't have to go out of their way to "fix" the issue when using NextJS or Gatsby and could flick an option on instead :)

@astoilkov
Copy link
Owner

The current state is — I like the idea and I am willing to implement it. I will post here when I start working on it.

@chrbala
Copy link

chrbala commented Nov 20, 2021

I have form state persisted with useLocalStorage + SSR. The server was initializing the form with the initial state, and the form wasn't updating until the first interaction, since React wasn't re-rendering until a client-side state change.

This could probably be fixed by a forced re-render when the component mounts, but I chose instead to lazily wait to render the whole element when the page loaded.

I did the following:

const useIsServer = () => {
  const [isServer, setIsServer] = useState(typeof window === 'undefined');
  useEffect(() => {
    if (isServer) setIsServer(false);
  }, [isServer]);
  return isServer;
};

const MyComponent = () => {
  const isServer = useIsServer();
  return <>
    {/* stuff */}
    {isServer ? null : <LazyComponent />}  
  </>
}

This concept could be combined into the main API. I think this type of solution may work without causing an extra render on the client.

The API could be:

useLocalStorageState(name, initialValue?, serverValue?);

Where serverValue is the value returned on the server, replaced with initialValue after the page loads. This puts the decision of what exactly to do with the server into userland, since returning nothing may not be the preferred option in all cases.

I have those marked as optional for backwards compatibility. I do see advantages to making serverValue required. It's a bit of a footgun to not have a default, since the bugs with inconsistency are pretty time consuming to debug. Perhaps it would be best to make the server value required in the next major version.

There is also the case of React Native, which I didn't test. I'm not that familiar with RN development, so I'm not sure if it's important to include in an API like this.

@astoilkov
Copy link
Owner

@chrbala You are proposing instead of having { ssr: boolean } property (the previously proposed solution) to have a serverValue as a third parameter. Am I understanding correctly?

If yes, I am wondering why not just use initialValue as the serverValue? Maybe there is a scenario that this won't be logical but I can't think of such. Do you have an example in mind?

@chrbala
Copy link

chrbala commented Nov 23, 2021

Yeah, that's what I was thinking. That may not be the optimal API though. The main thing for me is that it's not clear exactly what { ssr: boolean } does, or even when I'm supposed to mark the flag. Some ambiguity:

  • Do I mark it as true if I'm using SSR?
  • What happens if I mark it as false, but I'm actually using SSR? What happens if I mark it as true, but I'm not using SSR?
  • Can we use a component in multiple environments safely? What if it needs to run both SSR in some places and non-SSR in others?

The main things I'd look for are:

  1. Does not cause SSR inconsistencies when correctly configured.
  2. Does not create a footgun for people who haven't thought in detail about the server case. i.e. good defaults
  3. Does not render an extra pass for client-side only cases. The default case could be an extra render, but ideally wouldn't have to be.
  4. Can run safely in different environments without custom configuration.

@astoilkov
Copy link
Owner

Nice. I believe in the idea that the right questions lead you to the correct answers. Those are very good questions. Thank you!

@astoilkov
Copy link
Owner

I started working on this.

If you are one of the people who is waiting for this, you can upvote this comment.

@astoilkov
Copy link
Owner

I've created a draft implementation — 2bcbc52.

However, I will need some feedback. Any willing to take a look at it? I am wondering about:

  • Can the names of the methods be improved?
  • Do you agree that the methods are separate? This approach improves both performance and bundle size.
  • Does the implementation makes sense? There is a simple optimization in the code that doesn't cause a second render if server and client values are the same.

@nxtman123
Copy link

nxtman123 commented Jan 8, 2022

@astoilkov I tried the draft implementation in my project just to test it and was able to get it working

  • I can't think of better names, but they're not amazing either.
  • Are the bundle size and performance improvements just for client-only apps over those apps that use the ssr versions?
  • I see the optimization you mentioned. The code makes sense to me.

I know this is a draft, but src/createSsrLocalStorageStateHook.ts didn't give me the same type signature as createLocalStorageHook. I was able to get the right signature with this, but it fails type check on ...args

export const createSsrLocalStorageStateHook: typeof createLocalStorageStateHook = (...args) =>
  withSsr(createLocalStorageStateHook(...args));

Really cool to find you working on this the same day I ran into the mismatch problem.

@astoilkov
Copy link
Owner

I can't think of better names, but they're not amazing either.

Yes, I will try to think of better ones.

Are the bundle size and performance improvements just for client-only apps over those apps that use the ssr versions?

Yes. For the apps that don't use server-side rendering.

I see the optimization you mentioned. The code makes sense to me.

Perfect. Thanks for this and for all the feedback.

I know this is a draft, but src/createSsrLocalStorageStateHook.ts didn't give me the same type signature as createLocalStorageHook. I was able to get the right signature with this, but it fails type check on ...args

Yes. I will fix this.

@astoilkov
Copy link
Owner

🎉 🎉 🎉

I'm ready and I've published a pre-release of the new version. The new version is a complete rewrite. I will explain in detail why I decided to make the change after I receive some feedback. The new version can be installed by running npm install use-local-storage-state@next and the documentation can be found at https://github.com/astoilkov/use-local-storage-state/tree/future.

Any feedback is very much welcome. Thanks!

@astoilkov
Copy link
Owner

I released 14.0.0 with hydration mismatch support. The library is now 30% smaller in size as well.

I hope this release makes this local storage hook the most popular 😇.

Closing this issue. Thanks to all who participated and helped.

@MichaelAllenWarner
Copy link

Why the approach with hasMouted doesn't use useLayoutEffect()? Wouldn't this update faster and the user see less of a flicker (or maybe even no flicker)?

Good point. That does avoid the flicker, but results in a warning due to useLayoutEffect being run the server [0]. I was able to work around that by using useEffect on the server instead. In case you're interested, this I'm now handling it in my code now.

// use-has-hydrated.js
import { useState, useEffect, useLayoutEffect } from "react";

function useHasHydrated(beforePaint = true) {
  const [hasHydrated, setHasHydrated] = useState(false);

  // To reduce flicker, we use `useLayoutEffect` so that we return value
  // before React has painted to the browser.
  // React currently throws a warning when using useLayoutEffect on the server so
  // we use useEffect on the server (no-op) and useLayoutEffect in the browser.
  const isServer = typeof window === "undefined";
  const useEffectFn = beforePaint && !isServer ? useLayoutEffect : useEffect;

  useEffectFn(() => {
    setHasHydrated(true);
  }, []);

  return hasHydrated;
}

// use-last-provider.js
const defaultValue = "facebook";
const useLastProviderBase = createLocalStorageStateHook("lastAuthProvider", defaultValue);

const useLastProvider = () => {
  const [lastProvider, setLastProvider] = useLastProviderBase();
  const hasHydrated = useHasHydrated();
  // We only return the stored provider value after the app has hydrated
  // so that it's undefined on both the server and (initially) the client.
  // This allows React to properly hydrate from server and the component
  // calling this hook doesn't have to worry about it.
  return [hasHydrated ? lastProvider : defaultValue, setLastProvider];
};

@gragland

That's brilliant and seems to eliminate the flicker altogether on initial render.

I'm looking at making the following React Context and wrapping an entire SSG NextJS app in its Provider:

const IsHydratedContext = createContext(false);

export const IsHydratedProvider = ({ children }: { children: ReactNode }) => {
  const [isHydrated, setIsHydrated] = useState(false);

  const useEffectFn =
    typeof window === 'undefined' ? useEffect : useLayoutEffect;

  useEffectFn(() => {
    setIsHydrated(true);
  }, []);

  return (
    <IsHydratedContext.Provider value={isHydrated}>
      {children}
    </IsHydratedContext.Provider>
  );
};

export const useIsHydrated = () => useContext(IsHydratedContext);

Solves the server-vs-client thing efficiently—i.e., globally, immediately, and in a way that forces only the Context-subscribed components to re-render—and the hook doubles as a convenient "is-JS-enabled" flag, making it easy to improve the UX for users who have JS disabled.

The useLayoutEffect() looks like the secret sauce I'd been missing, so I'm wondering: have you encountered any problems with using it this way? or has it been smooth sailing?

@MichaelAllenWarner
Copy link

Hm, I think I spoke too soon: the useLayoutEffect only eliminated the flicker in development, not in production (presumably because of the time required to download and execute the JS). Still, it's probably the way to go if it doesn't cause problems.

@astoilkov
Copy link
Owner

astoilkov commented Mar 27, 2022

Did you see that there is an ssr property now that handles all this automatically?

CleanShot 2022-03-27 at 17 25 06@2x

@MichaelAllenWarner
Copy link

@astoilkov Sorry for the confusion—yes, I saw that part of the discussion, but I'm not actually using your hook. I just stumbled on this thread when researching the problem. My use-case is setting up a single app-wide flag for checking server-vs-client on an SSG NextJS site.

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

No branches or pull requests

6 participants