Skip to content

[example] Avoid double rendering in the Remix example #30366

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 3, 2022
Merged

[example] Avoid double rendering in the Remix example #30366

merged 3 commits into from
Jan 3, 2022

Conversation

@mnajdova
Copy link
Member

@mnajdova mnajdova commented Dec 22, 2021

Resolves #29952 (comment)

The goal is to avoid double rendering on the server. It was solved by injecting the emotion style tag after a specific meta tag in the head. It is similar to the new insertionPoint option recently added in emotion - emotion-js/emotion#2521

I could use some help testing out, to make sure I didn't miss anything.

@mui-pr-bot
Copy link

mui-pr-bot commented Dec 22, 2021 •
edited
Loading

No bundle size changes

Generated by 🚫 dangerJS against c3f6684

mnajdova
mnajdova commented Dec 22, 2021
View reviewed changes
@mnajdova mnajdova marked this pull request as ready for review December 22, 2021 15:53
@mnajdova mnajdova mentioned this pull request Dec 22, 2021
Merged
@mnajdova mnajdova requested a review from a team December 24, 2021 07:52
@jansedlon
Copy link
Contributor

jansedlon commented Dec 27, 2021

Tried replacing my solution that has double rendering with this one and I'm getting

Prop className did not match. Server: "MuiFormControl-root MuiFormControl-fullWidth makeStyles-root-2 makeStyles-root-6 MuiTextField-root makeStyles-textField-1 makeStyles-textField-5 css-wb57ya-MuiFormControl-root-MuiTextField-root" Client: "MuiFormControl-root MuiFormControl-fullWidth makeStyles-root-2 makeStyles-root-8 MuiTextField-root makeStyles-textField-1 makeStyles-textField-7 css-wb57ya-MuiFormControl-root-MuiTextField-root"

Not sure on whose side the problem is

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 27, 2021 •
edited
Loading

@jansedlon The class names mismatch is from @mui/styles. This package is for legacy usages. The examples are no longer covering it. So what you experience is expected.

Should we handle #30213 (comment) here?

@jansedlon
Copy link
Contributor

jansedlon commented Dec 27, 2021 •
edited
Loading

@oliviertassinari You're right. I had a few issues with css order so I just added prepend: true to cache and switched positions of <Links /> and <meta name="emotion-insertion-point" content="emotion-insertion-point" /> and so far except one error Expected server HTML to contain a matching <link> in <head>.(which doesn't affect anything visible) it's working well. The performance is the same (in dev env)

Edit: Looks like that links (fonts and tailwind) are duplicated in head (but according to devtools fetched only once, so that's good)

siriwatknp
siriwatknp approved these changes Jan 3, 2022
View reviewed changes
Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Better than double rendering.

@mnajdova
Copy link
Member Author

mnajdova commented Jan 3, 2022

Should we handle #30213 (comment) here?

Will handle it in a follow up, thanks for linking it.

@mnajdova mnajdova merged commit a0394af into mui:master Jan 3, 2022
@mnajdova mnajdova mentioned this pull request Jan 3, 2022
Merged
1 task
@oliviertassinari oliviertassinari added examples Relating to /examples. performance labels Jan 3, 2022
@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 3, 2022

Nice, this looks great!

@oliviertassinari oliviertassinari mentioned this pull request Jan 3, 2022
Closed
wladimirguerra pushed a commit to wladimirguerra/material-ui that referenced this pull request Feb 2, 2022
@mnajdova @wladimirguerra
[example] Avoid double rendering in the Remix example (mui#30366)
cbffdb3
@ShuPink ShuPink mentioned this pull request Jun 1, 2022
Open
2 tasks
@mnajdova mnajdova mentioned this pull request Jun 1, 2022
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@siriwatknp siriwatknp siriwatknp approved these changes

@oliviertassinari oliviertassinari Awaiting requested review from oliviertassinari

Assignees

No one assigned

Labels

examples Relating to /examples. performance

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mnajdova @mui-pr-bot @jansedlon @oliviertassinari @siriwatknp