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

className not being correctly applied after server render #14601

Closed
lax4mike opened this issue Jun 6, 2019 · 5 comments
Closed

className not being correctly applied after server render #14601

lax4mike opened this issue Jun 6, 2019 · 5 comments

Comments

@lax4mike
Copy link

lax4mike commented Jun 6, 2019

Description

I made a minimal example repo that you can find here: https://github.com/lax4mike/gatsby-bug
Is this a bug with server rendering in Gatsby? or server rendering with React in general?

Steps to reproduce

  1. Open http://mikelambert.me/gatsby-bug/ in a browser wider than 600px.
  2. Notice that the background is green and the text reads CONTAINER--LARGE (should be blue).
    • The text was server rendered as container--small (should be green) (view source to see), and is correctly being updated to CONTAINER--LARGE (should be blue)
    • The color/class is incorrect! The incorrect className of container--small is in the DOM. However, notice the correct className of container--large is logged to the console.

(see https://github.com/lax4mike/gatsby-bug/blob/master/README.md)

Expected result

When opening http://mikelambert.me/gatsby-bug/ in a browser wider than 600px the background should be blue because the class container--large is applied.

Actual result

When opening http://mikelambert.me/gatsby-bug/ in a browser wider than 600px the background is be green because the incorrect class of container--small is applied.

Environment

System:
OS: macOS 10.14.1
CPU: (4) x64 Intel(R) Core(TM) i7-4558U CPU @ 2.80GHz
Shell: 3.2.57 - /bin/bash
Binaries:
Node: 10.14.2 - /usr/local/bin/node
npm: 6.9.0 - /usr/local/bin/npm
Languages:
Python: 3.7.2 - /usr/local/opt/python/libexec/bin/python
Browsers:
Chrome: 74.0.3729.169
Firefox: 65.0.1
Safari: 12.0.1
npmPackages:
gatsby: ^2.8.5 => 2.8.5
npmGlobalPackages:
gatsby-cli: 2.6.5

@DSchau
Copy link
Contributor

DSchau commented Jun 7, 2019

I believe this to be a hydration issue. ReactDOM has trouble patching text content (e.g. attributes) that differ between the server-rendered content vs. what's now available on the content.

Specifically, I recommend checking out facebook/react#13260 for more context on the underlying issue. Specifically, the below passage from ReactDOM Hydrate documentation is very illustrative:

If you intentionally need to render something different on the server and the client, you can do a two-pass rendering. Components that render something different on the client can read a state variable like this.state.isClient, which you can set to true in componentDidMount(). This way the initial render pass will render the same content as the server, avoiding mismatches, but an additional pass will happen synchronously right after hydration. Note that this approach will make your components slower because they have to render twice, so use it with caution.

A janky solution (that works) is to pass a key that differs between server/client. Note that the React tree will be re-mounted/rendered between server and client, and I'd generally recommend just using CSS Media queries whenever you're able to to side-step this general issue.

import React,  { useState, useEffect } from "react"
import useMatchMedia from "../useMatchMedia.js"
import "../styles.css"

export default () => {
  const [isClient, setClient] = useState(false)
  const isLarge = useMatchMedia("(min-width: 600px)")

  useEffect(() => {
    setClient(true)
  }, [])

  const sizeClass = isLarge ? "container--large" : "container--small"
  const key = isClient ? `client` : `server`

  return (
    <div className={`container ${sizeClass}`} key={key}>
      {isLarge
        ? "CONTAINER--LARGE! (should be blue)"
        : "container--small (should be green)"}
    </div>
  )
}

Going to close as answered, and thanks for the great question! If we can help further, let us know!

@DSchau DSchau closed this as completed Jun 7, 2019
@lax4mike
Copy link
Author

lax4mike commented Jun 14, 2019

Hi @DSchau, thanks for the help!

I was able to fix my useMatchMeda by defaulting my matches state to false, and moving the mediaQueryList definition into the effect. Having the false default ensures that the server and the first render on the client match (which I think was the cause of the problem). The effect code won't be executed on the server (so it won't choke on the window), and will execute when the client loads to update the value based on the window width.

import { useState, useEffect } from "react"

/**
 * useMatchMedia
 *
 * usage:
 *   const matches = useMatchMedia("(min-width: 900px)")
 *   matches will be true or false
 *
 * the format of the string is important, eg, needs ()'s
 * see https://developer.mozilla.org/en-US/docs/Web/API/Window/matchMedia
 * @param  {String} media : media query to match
 * @return {Boolean} true if it matches, false if it doesn't
 */
export default function useMatchMedia(media) {
  const [matches, setMatches] = useState(false)

  // define mediaQueryList inside effect because of server rendering/hydration
  // we need to render again when the client loads
  useEffect(() => {
    const mediaQueryList = window.matchMedia(media)
    const handleMatchChange = event => setMatches(event.matches)

    setMatches(mediaQueryList.matches)
    mediaQueryList.addListener(handleMatchChange)

    return () => {
      mediaQueryList.removeListener(handleMatchChange)
    }
  }, [media])

  return matches
}

EDIT: For reference, here is the origianal useMatchMedia:

export default function useMatchMedia(media) {
  // checking if window exists for server rendering
  const mediaQueryList =
    typeof window !== "undefined" ? window.matchMedia(media) : {}

  const [matches, setMatches] = useState(mediaQueryList.matches)

  useEffect(() => {
    const handleMatchChange = event => setMatches(event.matches)

    setMatches(window.matchMedia(media).matches)
    mediaQueryList.addListener(handleMatchChange)

    return () => {
      mediaQueryList.removeListener(handleMatchChange)
    }
  }, [matches, media, mediaQueryList])

  return matches
}

@eyalroth
Copy link
Contributor

eyalroth commented Aug 8, 2019

@DSchau This is a major bug and should be addressed better by providing a fix / builtin workaround or a very clear documentation about it.

Why is this such a major bug?

  1. It is hard to detect, both because (a) it will only show on a built site, not on a development server, and because (b) it's only the className / attributes which do not update while the content does (which may very well give the notion that the attributes do too).
  2. More importantly, the workaround does not work for nested components, which affects a pretty common use-case.

It is quite common to have a layout component which wraps the entire page with a div and sets the theme as className, while the theme can be changed by nested components (say, a button) via a React context:

const ThemeContext = React.createContext({theme: themes.dark, toggleTheme: () => {}});

class ThemeProvider extends React.Component {
    constructor(props) {
        super(props)

        this.toggleTheme = () => {
            this.setState(state => ({
                theme:
                    state.theme === themes.dark
                        ? themes.light
                        : themes.dark,
            }));
        };
        
        this.state = {
            theme: themes.light,
            toggleTheme: this.toggleTheme,
        }
    }

    render() {
        return <ThemeContext.Provider value={this.state}>{this.props.children}</ThemeContext.Provider>
    }
}

class Layout extends React.Component {
    render() {
        return (
            <ThemeProvider>
                <ThemeContext.Consumer>
                    { ({theme}) => (
                        <div className={`container ${theme.css}`}>{this.props.children}</div>
                    )}
                </ThemeContext.Consumer>
            </ThemeProvider>
        )
    }
    
    componentDidMount() {
        this.forceUpdate() // this is equivalent to the useEffect workaround with React functions
    }
}

This is basically a combination of React's guide on Context and Gatsby's layout plugin guide.

The button will be able to update the theme properly, but once the page is refreshed in the browser the className of the "container div" will return to the original (resetting the theme).

@kriskate
Copy link

I've encountered this exact issue (client className should be different, in conformity with the window's width).

useMatchMedia is, surely, a good workaround for a client-rendered app, but with SSR, before the actual client code hydrates the DOM, the user (of, for example a low-end network) would be shown the SSR'd version. (so it would flicker)

I've fixed this issue by using actual media queries (via mixins in scss for an app-wide solution), because once the browser has rendered the SSR markup, the stylesheet would automatically apply the style (and not wait for the client to append a device-specific className.

$mobile: 768px;
$tablet: 1024px;
$desktop: 1025px;

@mixin device-type($media) {
  @if $media == mobile {
    @media only screen and (max-width: $mobile) {
      @content;
    }
  } @else if $media == tablet {
    @media only screen and (max-width: $tablet) {
      @content;
    }
  } @else if $media == desktop {
    @media only screen and (min-width: $desktop) {
      @content;
    }
  }
}

... and used it in my component's css as

.my-component-classname {
    width: 50%;

    @include device-type(mobile) {
      width: 100%;
  }
}

The other solution I've been working on was implying the server to take the user agent and at SSR append a device-specific classname accordingly (via device).
This approach would work for actual devices (or Chrome's devTools emulator), but if the app is opened on a desktop, with the window's width let's say half the screen, the above described "flickering" issue would still be there.

@blainekasten
Copy link
Contributor

#25729

MaxwellKendall added a commit to MaxwellKendall/maxwellkendall.com that referenced this issue Feb 25, 2021
MaxwellKendall added a commit to MaxwellKendall/maxwellkendall.com that referenced this issue Feb 25, 2021
MaxwellKendall added a commit to MaxwellKendall/maxwellkendall.com that referenced this issue Feb 25, 2021
MaxwellKendall added a commit to MaxwellKendall/maxwellkendall.com that referenced this issue Feb 25, 2021
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

5 participants