Skip to content

[docs] Document emotion migration breaking changes#24229

Merged
oliviertassinari merged 4 commits intomui:nextfrom
luminaxster:patch-1
Jan 3, 2021
Merged

[docs] Document emotion migration breaking changes#24229
oliviertassinari merged 4 commits intomui:nextfrom
luminaxster:patch-1

Conversation

@luminaxster
Copy link
Contributor

@luminaxster luminaxster commented Jan 3, 2021

@mui-pr-bot
Copy link

mui-pr-bot commented Jan 3, 2021

No bundle size changes

Generated by 🚫 dangerJS against 6c61127

@oliviertassinari
Copy link
Member

@luminaxster Which breaking change did you identify?

@oliviertassinari oliviertassinari changed the title jss to emotion breaking changes [docs] Document Emotion migration breaking changes Jan 3, 2021
@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation. label Jan 3, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 3, 2021

@luminaxster I have tried to improve the changes to mention the breaking change coming from v5.0.0-alpha.17. Could you have a look to see if it matches your expectation? Thanks

@Jack-Works
Copy link
Contributor

Cool to see that. But I have a question. Now the recommended order is JSS > emotion, this order seems not breaking our app currently. But what if I use a JSS based (not migrated yet) MUI component and style it with emotion? By this order, my customization will not work.

@oliviertassinari
Copy link
Member

But what if I use a JSS based (not migrated yet) MUI component and style it with emotion? By this order, my customization will not work.

@Jack-Works Yeah, you are right. We have a couple of visual regression tests in our codebase that is broken because of this (I believe). You could use import { styled } from '@material-ui/core/styles' (JSS) then switch to experimentalStyled (emotion). The two should have the same API.

@mbrookes
Copy link
Member

mbrookes commented Jan 3, 2021

It seems like this belongs in the migration guide even more-so than the change log.

@oliviertassinari
Copy link
Member

It seems like this belongs in the migration guide even more-so than the change log.

This is most relevant during the phase where the codebase still mix JSS and emotion. We have it in the migration guide: https://next.material-ui.com/guides/migration-v4/#styled-engine.

Co-authored-by: Matt <github@nospam.33m.co>
@oliviertassinari oliviertassinari dismissed mbrookes’s stale review January 3, 2021 16:05

suggestion approved

@luminaxster
Copy link
Contributor Author

luminaxster commented Jan 3, 2021

I followed your changes @oliviertassinari, It works like a charm now. Since that was the first migrated component and I was not customizing it, I lazily skipped over the emotion details. Following on @mbrookes, indeed, what is missing is mentioning the guide when components are migrated, a compromise may be to add a note like this each time a component has been migrated to emotion:
[Emotion] [More details on setting up your migration to emotion (if you haven't already)](https://next.material-ui.com/guides/interoperability/#css-injection-order) in the documentation..
It may keep the "troubleshooting" of the migration changes self-contained as the changelogs pile up.

Thank you so much for your time and patience! =P.

@oliviertassinari
Copy link
Member

@luminaxster Thanks for the confirmation. Yeah, this migration period will be challenging to handle. The good news that it will only get better over time.

@oliviertassinari oliviertassinari changed the title [docs] Document Emotion migration breaking changes [docs] Document emotion migration breaking changes Jan 3, 2021
@oliviertassinari oliviertassinari merged commit 5423c15 into mui:next Jan 3, 2021
@luminaxster luminaxster deleted the patch-1 branch January 3, 2021 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to the documentation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants