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

useMediaQuery is not SSR safe? #394

Closed
bryantcodesart opened this issue Nov 2, 2023 · 4 comments · Fixed by #445
Closed

useMediaQuery is not SSR safe? #394

bryantcodesart opened this issue Nov 2, 2023 · 4 comments · Fixed by #445
Labels
bug Something isn't working

Comments

@bryantcodesart
Copy link

bryantcodesart commented Nov 2, 2023

I might be doing something wrong or misunderstanding, but I recently started using this hook in Next and immediately ran into SSR issues.

https://usehooks-ts.com/react-hook/use-media-query

I believe the culprit is the getMatches behavior. There is a window check in the spirit of SSR, cool! When this runs on the server we won't call the nonexistent window object. We'll return false as a default. So far so good. BUT because getMatches is run on the useState initialization, it will run first thing on the client initial render, where the window object DOES exist.

So, on the server, the state initializes with a default state of false. On the client, it initializes with the actual media query match state. So, if that's not false, hydration error!

One way to solve: Perhaps it should return the default the first time no matter what, and only update in an effect?

// Add defaultSsrValue
export function useMediaQuerySsr(query: string, defaultSsrValue = false) {
  // Always return default value first time, dont use getMatches
  const [matches, setMatches] = useState<boolean>(defaultSsrValue);

  const getMatches = (query: string): boolean => {
    if (typeof window !== "undefined") {
      return window.matchMedia(query).matches;
    }
    return defaultSsrValue;
  };

  function handleChange() {
    setMatches(getMatches(query));
  }

  useIsomorphicLayoutEffect(() => {
    const matchMedia = window.matchMedia(query);

    // Triggered at the first client-side load and if query changes
    handleChange();

    // Listen matchMedia
    if (matchMedia.addListener) {
      matchMedia.addListener(handleChange);
    } else {
      matchMedia.addEventListener("change", handleChange);
    }

    return () => {
      if (matchMedia.removeListener) {
        matchMedia.removeListener(handleChange);
      } else {
        matchMedia.removeEventListener("change", handleChange);
      }
    };
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [query]);

  return matches;
}

Of course, this might create issues for non-SSR applications that would like to get the correct value on first render. So perhaps such a strategy should be opt-in.

OR. Am I missing something really silly?

@Ben888GitHub
Copy link

Can you put these codes into an individual Client component, and then dynamically import this component into your page This can probably solve hydration error.

@bryantcodesart
Copy link
Author

bryantcodesart commented Nov 7, 2023

Sure that's one way! It's not my favorite because I'd prefer to not add additional network requests for code I KNOW I'll be using above the critical fold. I like dynamic importing for... things that should be imported dynamically! ;) Rather than as a workaround. Also the complexity of dynamic imports can balloon real quick, especially if you use this as part of another hook that gets imported in a lot of places (and a useMediaQuery hook is VERY likely to be a building block for another custom hook).

There are about 100 good ways to solve this problem--including your proposed solution--but I just wanted to point out that it is a problem--and it appears in a few hooks in this library. If you use this hook unmodified in e.g. Next.js, you get hydration errors. And they are of the sort that a more junior-level dev would be VERY confused debugging. (Ditto any other hook in this lib deferring to the window object in the useState initialization) Just thought you'd want to know, since you've done such a commendable job positioning this library as 1. typesafe 2. SSR-safe!

What I love about this library philosophically is how accessible you've made the source code in the documentation. AWESOME. So I just fork the hooks and change them to my liking when little things like this come up. And that's fine for me. I've forked and made my own version of most of these hooks. They are AMAZING templates to build on--and I just customize for project needs!

But I thought maybe you'd just want to know about this issue and to mention it in your documentation!

(This tension between browser APIs and SSR is obviously a fact of life. More often than not, I try to solve these things with CSS for this reason. But sometimes you can't avoid a useMediaQuery! And for the template, I'm super grateful!!)

@juliencrn
Copy link
Owner

Thanks a lot @bryantcodesart 👍 You're right.
I've implemented your fix in both useScreen and useMediaQuery there #445

Ps: I love your personal website, it's pretty cool ✌️

@bryantcodesart
Copy link
Author

@juliencrn Oh heck ya! I love this library. So glad I could contribute in any small way! And psyched you dig my site! lololol. These hooks def make some appearances in there! :)

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

Successfully merging a pull request may close this issue.

3 participants