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

Dynamic global styles with SSR issue #2158

Closed
zce opened this issue Dec 4, 2020 · 8 comments · Fixed by #2334
Closed

Dynamic global styles with SSR issue #2158

zce opened this issue Dec 4, 2020 · 8 comments · Fixed by #2334

Comments

@zce
Copy link

zce commented Dec 4, 2020

Current behavior:

style tags generated by <Global /> components appear in front of style tags generated by SSR:

image

As a result, the dynamically calculated global styles cannot override the styles in the SSR:

image

To reproduce:

https://codesandbox.io/s/snowy-violet-tq704

Expected behavior:

The style generated by SSR should appear first:

image

Environment information:

  • react version: 17
  • @emotion/react version: 11
@marcel0ll
Copy link

marcel0ll commented Dec 17, 2020

I am having similar problems while using gatsby + emotion. Is there a way to control de order that emotion style tags are injected into the HTML?

@Andarist
Copy link
Member

If you would not use the extractCritical API and just let the default approach to do its work then it would work correctly as you expect it to do so. That being said the reported situation is not ideal and I'm thinking about how it can be fixed. It's also highly related to the problem reported here: #2049

I am having similar problems while using gatsby + emotion. Is there a way to control de order that emotion style tags are injected into the HTML?

There is only prepend option for caches. You could work around this issue by implementing an alternative version of extractCritical. The basic requirement to make this work is to split regular rules and global ones in the rendered SSR output.

@zce
Copy link
Author

zce commented Dec 20, 2020

At present, my solution is to exclude <Global /> from the CacheProvider.

export const wrapRootElement = ({ element }) => (
  <ThemeProvider theme={theme}>
    <CacheProvider value={global}>
      <Global styles={styles} />
    </CacheProvider>
    <CacheProvider value={cache}>{element}</CacheProvider>
  </ThemeProvider>
)

<Global /> will generate a style tag during SSR, and it will move to the front of the first emotion style tag when running

@oliviertassinari
Copy link

oliviertassinari commented Jan 16, 2021

I have found the same issue. Here is a minimal reproduction (without MUI): https://github.com/oliviertassinari/mui-repro-global-styles. The bug was surfaced in mui/material-ui#24440 (comment).

@Andarist
Copy link
Member

@oliviertassinari I believe it would be best to create extractCritical2 and deprecate the old extractCritical. The new one should just return a list of objects instead of a flat object. That list would contain a single entry for "regular" styles and as many entries as global styles. Each of those would have to be converted into a style element by the caller (in a similar way how one already does this, just right now there is only a single style element to be generated).

This would also require some changes in @emotion/css - right now it assumes a single sheet but it would have to start creating new sheets for each injectGlobal call. This would have an impact on implementations of some API method like flush - it shouldn't be overly complicated for those APIs to be adjusted though. This would also open a door for a new possibility - removing injected global styles (when using @emotion/css).

My schedule is quite busy lately so it might take me a while before I could get to this - if your team could help out with this, that would be quite great.

@oliviertassinari
Copy link

oliviertassinari commented Jan 17, 2021

The workaround proposed by @zce seems to do it.

    <React.Fragment>
      <Global
        styles={{
          body: {
            backgroundColor: darkmode ? "#000" : "#fff",
          },
        }}
      />
      <CacheProvider value={cache}>
        <Component {...pageProps} />
        <button onClick={() => setDarkMode((prevState) => !prevState)}>
          switch to {darkmode ? "light" : "dark"} mode!
        </button>
      </CacheProvider>
    </React.Fragment>

The only downside is one invalid HTML markup. It's OK-ish for a temporary solution. The global style can be rendered before any other content of the body. The warning only happens once, not flooding / burying the other important potential errors too much. However, in the long term, we need to fix it.

Capture d’écran 2021-01-17 à 14 57 36


@Andarist We will keep it in mind, thanks for taking the time to share pointers on how you envision the solution. Maybe we can come back to it once we release v5 stable.

@oliviertassinari
Copy link

oliviertassinari commented Apr 2, 2021

cc @mnajdova in case you want to work on #2158 (comment)

@mnajdova
Copy link
Contributor

mnajdova commented Apr 2, 2021

cc @mnajdova in case you want to work on #2158 (comment)

I can start working on this next week 👍

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

Successfully merging a pull request may close this issue.

5 participants