Skip to content

"Advanced approach" SSR with Next broken with react-dom 18 #2725

New issue
New issue
Closed
Closed
"Advanced approach" SSR with Next broken with react-dom 18#2725
Assignees
Andarist
Labels
bugneeds triage
@garronej

Description

@garronej
garronej
opened on Apr 16, 2022 · edited by garronej
Contributor

Hi,
Again, thank you for your work on Emotion, it's a fantastic library.

UPDATE: Scroll down to this message, it better describe the issue.

Activity

garronej
added
bug
needs triage
on Apr 16, 2022
garronej
mentioned this on Apr 16, 2022
srmagura

srmagura commented on Apr 16, 2022

@srmagura
srmagura
on Apr 16, 2022
Contributor

Emotion inserts styles differently in React 18 to be compatible with React's new SSR streaming capabilities.

Could you explain what the expected behavior vs. actual behavior is for Emotion in the context of React 18? Ideally you would also provide a minimal reproducible example.

BTW, I tried running your project on Windows but could not because of the Unix-specific scripts.

garronej
added 2 commits that reference this issue on Apr 17, 2022

Make test app runnable on Windows #78 emotion-js/emotion#2725

cd8fef9

Make test app runnable on Windows #78 emotion-js/emotion#2725

3617737
garronej

garronej commented on Apr 18, 2022

@garronej
garronej
on Apr 18, 2022 · edited by garronej
ContributorAuthor

Hi @srmagura thank you for your answer,

As I am sure you know, MUI is using emotion.
With MUI, 0-config SSR isn't supported, to enable SSR, MUI users are expected to explicitly provide an emotion cache and manually insert the <style> tags in the <head> as described in the "advanced approach" section of the emotion documentation.
You can find here a canonical setup for MUI + Next.js. This approach no longer works with react 18.

Reproduce

I created a minimal repo to exhibit what is no longer working as it used to.
Code: https://github.com/garronej/ssr-react-18-next

Fist disable JavaScript

git clone https://github.com/garronej/ssr-react-18-next
cd ssr-react-18-next
yarn
yarn build
yarn start
#heads over to http://localhost:3000

You'll see:
image

Now, if you downgrade to react-dom 17.0.2 you'll get:

image

What's happening

You can see this <style> tag inserted at the end of the <head> should take priority over color: read generated by css here.

image

The red string tag is not generated with react-dom 17 and causes the green style to be overwritten.

My 2 sence

If you run the app with yarn dev the bug happens only the very first render but we can see that with react-dom 18 unlike with react-dom 17 the _app.tsx is run twice and the first time it's before getInitialProps() of our custom _document.tsx has been called.

Screen.Recording.2022-04-18.at.02.22.21.mov

In the hope that a solution can be found,
Best

garronej
mentioned this on Apr 18, 2022
srmagura

srmagura commented on Apr 18, 2022

@srmagura
srmagura
on Apr 18, 2022
Contributor

@garronej Thank you so much, that really clears it up.

The "Advanced Approach" docs seem to be incomplete or incorrect for React 18 at the moment. Someone will need to do some testing with Next and MUI and either fix or add to that section in our docs.

Apart from the aforementioned documentation issue, I don't see anything Emotion is doing that is "wrong". My current feeling is that MUI is not compatible with the new streaming SSR, and that that is the underlying issue.

After reading the relevant code in Emotion (which is quite complex), I think Emotion will always insert <style> tags into the <body> when using React 18, even if the Advanced Approach is used and SSR streaming is not used. (I could be reading the code wrong though.) In React 17 when using the Advanced Approach, Emotion does not directly insert <style> tags during SSR, but our docs recommend you manually place them in the <head>.

Maybe we need to insert a toggle into Emotion to allow users to revert back to the React 17 behavior even when using React 18? I am thinking of this as a stopgap until MUI can fully support SSR streaming.

A lot of the discussion on mui/material-ui#24109 is likely relevant to this.

srmagura
assigned
Andarist
on Apr 18, 2022
Andarist
mentioned this on Apr 19, 2022
Andarist

Andarist commented on Apr 19, 2022

@Andarist
Andarist
on Apr 19, 2022
Member

I've created a Next.js issue about this here.

As a workaround you set clientSideEmotionCache.compat = true here. However, I think that the setup used in this repro case is slightly weird - you probably shouldn't use CacheProvider in your MyApp. When combined with Document.getInitialProps this will result in wrapping the whole thing twice with the CacheProvider and you don't pass the "outer" cache to the inner CacheProvider - I would suspect this leading to some unexpected results.

Andarist

Andarist commented on Apr 22, 2022

@Andarist
Andarist
on Apr 22, 2022
Member

The created issue was closed by this PR so I will assume that this should get fixed with the next Next release.

Andarist
closed this as completedon Apr 22, 2022
Andarist
mentioned this on Apr 29, 2022
garronej

garronej commented on May 3, 2022

@garronej
garronej
on May 3, 2022
ContributorAuthor

Hi,
Next 12.1.6 just released.
I confirm that it solves the issue.

himadrinath

himadrinath commented on May 12, 2022

@himadrinath
himadrinath
on May 12, 2022

looks like the problem arrived again in canary release.
can any one confirm that

garronej

garronej commented on May 12, 2022

@garronej
garronej
on May 12, 2022
ContributorAuthor

Hi @himadrinath,

I just tested with the current canary version,
This specific issues is fixed, there is no regression.

However, there are other issues, see the warning of tss-react:

🚨 WARNING: Next.js + React 18 -> Broken SSR.
This is not an issue specific to tss-react it affects MUI as a whole.
More specifically, there is currently no working configuration for Next.js + React 18 + Emotion advanced approach (which is the only strategy supported by MUI).
Follow the advancement of the related issues on the Next.js repo, on the MUI repo and on the TSS repo.

srmagura
mentioned this on May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugneeds triage

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    No branches or pull requests

      Participants

      @srmagura@garronej@Andarist@himadrinath

      Issue actions
        "Advanced approach" SSR with Next broken with `react-dom` 18 · Issue #2725 · emotion-js/emotion