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

[examples] change CreateEmotionCache to use insertionPoint #32104

Merged
merged 4 commits into from
Jul 14, 2022
Merged

[examples] change CreateEmotionCache to use insertionPoint #32104

merged 4 commits into from
Jul 14, 2022

Conversation

ANTARES-KOR
Copy link
Contributor

@ANTARES-KOR ANTARES-KOR commented Apr 2, 2022

Close #32068

@ANTARES-KOR ANTARES-KOR changed the title [examples] change createEmotionCache to use insertionPoint (#32068) [examples] change createEmotionCache to use insertionPoint Apr 2, 2022
@ANTARES-KOR
Copy link
Contributor Author

One thing is, It worked fine on ssr, nextjs, nextjs-with-typescript examples but not worked on remix-with-typescript example.

@mui-bot
Copy link

mui-bot commented Apr 2, 2022

No bundle size changes

Generated by 🚫 dangerJS against 9247722

@mnajdova
Copy link
Member

mnajdova commented Apr 4, 2022

Thanks for the contribution, will test it out and review it this week. I will also try to see why the remix example is not working.

@danilo-leal danilo-leal added the examples Relating to /examples label Apr 4, 2022
@mnajdova
Copy link
Member

mnajdova commented Apr 7, 2022

This doesn't work in all cases. I've build for production the nextjs project and when running it with disabled JavaScript (to test SSR), I am getting this (note the style tags being added before each element):

image

@ANTARES-KOR
Copy link
Contributor Author

ANTARES-KOR commented Apr 10, 2022

@mnajdova I took a look on it, but this bug(?) happens on master branch too (when disabled javascript on browser)

스크린샷 2022-04-10 오후 10 17 47

@mnajdova
Copy link
Member

It definitely should happen. Could be a regression from a recent PR. I will take a look at it next week. Thanks for catching it.

If you inspect our docs, for example - https://mui.com/material-ui/react-floating-action-button/ with disabled JavaScript you will see that all styles are in the head, there are no inline style tag before each element. This is how it should look like.

@ANTARES-KOR
Copy link
Contributor Author

@mnajdova I think this problem can be somewhat related to this issue..?

@mnajdova
Copy link
Member

Yes, could be, thanks for linking the issue. Let's wait for the stable nextjs release with the fix and test out again :)

@mnajdova mnajdova added the on hold There is a blocker, we need to wait label May 18, 2022
@mnajdova
Copy link
Member

Let's wait for the next version that contains the fix in vercel/next.js#36675 (comment) to be stable so that we can test the changes reliably.

@ANTARES-KOR
Copy link
Contributor Author

@mnajdova Tested with Next.js 12.2.0. And I can see that this <style/> tag regression is not happening anymore!

@mnajdova
Copy link
Member

mnajdova commented Jul 5, 2022

@mnajdova Tested with Next.js 12.2.0. And I can see that this <style/> tag regression is not happening anymore!

Nice! I am updating the nextjs to 12.2.0 in #33196 and we can continue this PR :)

@mnajdova
Copy link
Member

Just FYI, it's on my list for today :)

examples/ssr/server.js Outdated Show resolved Hide resolved
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for the patience :)

@mnajdova mnajdova changed the title [examples] change createEmotionCache to use insertionPoint [examples] change CreateEmotionCache to use insertionPoint Jul 14, 2022
@mnajdova mnajdova merged commit 1270c0b into mui:master Jul 14, 2022
daniel-rabe pushed a commit to daniel-rabe/material-ui that referenced this pull request Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples Relating to /examples on hold There is a blocker, we need to wait
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[examples] Change Next.js example's createEmotionCache from using 'prepend' to using 'insertionPoint'
4 participants