Material-ui: [useMediaQuery] defaultMatch is false before hydratationCompleted

[johannwagner]

Hey,

I just ran into an race condition with the useMediaQuery hook. If you use this in client side rendering, the first return is false. The component will rerender immediately due to the useEffect and will show the correct value.

I think, this is a bad behavior, because a useMediaQuery will most likely change the component, which causes a lot of unnecessary rerenders.

• [x] This is not a v0.x issue.
• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

It should render once and return the correct value.

Current Behavior 😯

It renders twice and returns the wrong value in the first place.

Steps to Reproduce 🕹

Link: https://stackblitz.com/edit/react-fcrunc

1. Load the site and open the console.
   Site shows the actual state.
   Console shows all history states.

Context 🔦

I want to create a defaultValue for a hook to decide, if I want to show a Sidebar or not.
My default value is false the entire time due to this bug.

Your Environment 🌎

| Tech | Version |
|--------------|---------|
| Material-UI | v3.9.1 |
| React | 16.7.0-alpha2 |
| Browser | Chrome 71 and Firefox 64 |
| TypeScript | 3.2.4 |

[johannwagner]

I just found my problem.
I need to set noSsr in the options object. This is not very intuitive and will lead to many bugs.
We may want to change this behavior.

noSsr is also missing on the typing for the options object.

[oliviertassinari]

@johannwagner We have many tests for the hook behavior. It was done on purpose. Let us know if you think that we can improve the documentation in any way. Do you want to fix the TypeScript definition?

[johannwagner]

@oliviertassinari

I just fixed the typing issue in https://github.com/mui-org/material-ui/pull/14339.

You may want to mention in the examples, not in the API description, that you have to enable noSsr to have a correct rendering. I think, most people does not read the API description, if they don't need to, like me. :speak_no_evil:

[oliviertassinari]

@johannwagner Thank you! :) Yes, it's eventually correct without noSsr. This logic is important for people doing server-side rendering. Now, the majority of the people 80% don't do SSR. So we are optimizing for the few. But, it's harder to fix a SSR mismatch than to fix a UI blink, hence the current tradeoff.

[fzaninotto]

I've been bitten by this, too.

I wonder why you can't be a bit more optimistic to avoid the double pass on the client side, by trying to run the queries even on the server side, but testing the presence of window first:

  // write
  const [matches, setMatches] = React.useState(() => {
    if (ssrMatchMedia) {
      return queries.map(query => ssrMatchMedia(query).matches);
    }
    return queries.map(query => window ? window.matchMedia(query).matches : defaultMatches);
  });

  // instead of
  const [matches, setMatches] = React.useState(() => {
    if (hydrationCompleted || noSsr) {
      return queries.map(query => window.matchMedia(query).matches);
    }
    if (ssrMatchMedia) {
      return queries.map(query => ssrMatchMedia(query).matches);
    }

    // Once the component is mounted, we rely on the
    // event listeners to return the correct matches value.
    return queries.map(() => defaultMatches);
  });

[fzaninotto]

Also, there is no way to make the difference between "the effect hasn't run yet" and "the media query doesn't match". This is a problem if I want to deliberately trigger an effect on mount for certain media queries (e.g. hiding a sidebar by default on small screens) but I don't want to make it on large screens to because the effect hasn't run yet...

[oliviertassinari]

I wonder why you can't be a bit more optimistic to avoid the double pass on the client side

@fzaninotto The result needs to match between the server and the client hydratation. There is no way around it. The first pass ensures the correct hydratation and the second pass ensure the eventually correct value.

there is no way to make the difference between "the effect hasn't run yet" and "the media query doesn't match"

Does setting defaultMatches: null solve the problem (it can be set in the theme)?

[fzaninotto]

can't you initialize hydrationCompleted to window to skip the double pass logic on the client side?

[oliviertassinari]

If we do it, we will lose the first pass that ensures the correct hydratation.

1. render on the server, window is not defined => false
2. render on the client, window is defined => true
3. run the hydratation, conflict between false and true.

Am I getting it wrong? We have added the noSrr option for working around the problem.

[fzaninotto]

I feel that there might be a solution, but I'm not sure. From what I understand, there are 3 cases:

• the code runs on the server. The developer MUST provide a ssrMatchMedia implementation. This implementation will be the one and only return of the hook.
• the code runs on the client following a server rendering. In that case, there is a ssrMatchMedia provided. the hook should first return the same value as the server (coming from ssrMatchMedia) for correct reconciliation, then return the real client-side value based on an event listener
• the code runs on the client with no prior server rendering. There is no ssrMatchMedia provided, so you don't need to worry about server and client matching. The hook can return the correct value using window directly, in a single pass.

I have troubles to understand the current code, so I must be missing something. That's why I may be asking stupid questions. Sorry about that.

[fzaninotto]

Also, I know about noSSR, but it looks like the wrong solution. Sacrificing SSR to get faster client.

[oliviertassinari]

The developer MUST provide a ssrMatchMedia implementation.

Most people are happy to ignore the problem, are not willing to do the extra mile for the benefit. It's not a requirement.

[fzaninotto]

Well, I didn't understand that it wasn't required while reading the documentation:

Server-side rendering
An implementation of matchMedia is required on the server, we recommend using css-mediaquery. We also encourage the usage of the useMediaQueryTheme version of the hook that fetches properties from the theme. This way, you can provide a ssrMatchMedia option once for all your React tree.

How can a developer pass a matchMedia if not via ssrMatchMedia?

Also, if SSR is 20% if the users but harms the 80% others, I think it's acceptable to ask a supplementary effort to SSR users (passing a ssrMatchMedia implementation).

[eps1lon]

the code runs on the client following a server rendering. In that case, there is a ssrMatchMedia provided. the hook should first return the same value as the server (coming from ssrMatchMedia) for correct reconciliation, then return the real client-side value based on an event listener

Agreed. window.matchMedia should not run in the render phase. It becomes tricky if you have two different builds for server and client. How does the client know what the server matched?

I think it's much safer to use an implementation that takes a defaultMatches (for the server) and runs the match in a passive effect. Otherwise your component will cause flashes. It's the same issue we have with our theming implementation in the docs: A hard coded theme is rendered on the server but on the client we switch out the persisted theme.

[fzaninotto]

Thanks for taking the time to answer my SSR-noob questions.

It becomes tricky if you have two different builds for server and client.

I don't understand that. The ssrMatchMedia should give the same result on the server and on the client hydration phase, to avoid a mismatch warning. Also, having two different implementations of ssrMatchMedia on the client and on the server seems to me as a corner case of what's already (SSR) a minority case. I wouldn't force a rerender on 80% of users for that use case.

I think it's much safer to use an implementation that takes a defaultMatches (for the server) and runs the match in a passive effect. Otherwise your component will cause flashes.

I don't understand that one either :(. Are you talking about something I should do with the current mui code, or something mui should do to improve the situation?

[eps1lon]

I'm talking about our implementation. We did a lot of premature optimization and shortcuts without actually profiling it. This is never a good idea.

But this is more internal ramblings. I'm not fully convinced yet that every component should/can be isomorphic. Especially a media query makes little sense on the server IMO. Maybe for a subset of the queries. I need to sit down at one point and step through every phase of SSR do check if all of the clever things we do aren't harmful.

[artegen]

There's a nasty case with this unfrequent option.
I needed to set drawer initial state open on desktop, and hidden on mobile. I did this:

 const matchesDown600 = useMediaQuery('(max-width:600px)');

 const initialOpen = matchesDown600 ? false : true;
 const [open, setOpen] = React.useState(initialOpen);

The issue with useMediaQuery jumping from one state to another hasn't bitten me before. But here it doesn't work, the initial state is set, and I don't see a good alternative to correct it when matchesDown600 is updated.

I also tried but didn't find a good way to go with a class component (you could use the componentDidMount). There you can't have this hook. This component is the layout, so no props passed from above.

Correcting to this works:

 const matchesDown600 = useMediaQuery('(max-width:600px)', { noSsr: true });

[JavaJamie]

I'm also seeing a similar issue which is causing an issue with appending the correct query to the URL. On browser mobile view, it seems to get the default breakpoint (desktop) before then getting the correct breakpoint (for mobile) on the second render. The problem is it assigns the desktop tab value which is wrong. I've tried adding the noSsr value but to no avail.

const tab = useMediaQuery(useTheme().breakpoints.down('sm'), { noSsr: true }) ? 'cardetails' : 'othercars';
<Button component={Link} to={`/cars/{carId}/tab?=${tab}`}>View car</Button>

I've also tried the following:

const matches = useMediaQuery(useTheme().breakpoints.down('sm'), { noSsr: true });
const [tab, setTab] = useState("othercars");

useEffect(() => {
    if (matches) {
        console.log("tab: ", tab)
        setTab("cardetails");
    }
}, [matches]);

return <Button component={Link} to={`/cars/{carId}/tab?=${tab}`}>View car</Button>

Either way, it seems whatever I do, it always uses the desktop tab ("othercars") even when on mobile/in a mobile view which should be using the "cardetails" one instead. This seems like a fairly standard thing so I'm not sure what I'm missing.

Thanks guys.

[ErnestBrandi]

I also have an issue with Next.js using getStaticProps and the fallBack: true option.
Using the noSsr option leads to the following warning : Layout was forced before the page was fully loaded and undefined behaviour.