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.

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

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

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

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 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

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

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