Skip to content

[examples] Update examples to use StyledEngineProvider#24489

Merged
mnajdova merged 4 commits intomui:nextfrom
mnajdova:feat/examples-styled-provider
Jan 28, 2021
Merged

[examples] Update examples to use StyledEngineProvider#24489
mnajdova merged 4 commits intomui:nextfrom
mnajdova:feat/examples-styled-provider

Conversation

@mnajdova
Copy link
Member

@mnajdova mnajdova commented Jan 18, 2021

Changes done in the PR:

  • Added usages of StyledEngineProvider with injectFirst option on all examples except the nextjs, nextjs-with-typescript and ssr. The reason why it was not added on these example is that the client and server need to share the same emotion cache. This requires custom cache object that can be imported from both places.

  • Updated the create-react-app-with-styled-components to have the same format and content as the other examples. Initially it was created with only the Slider component, so that we can test that the switch to styled-components is actually working. Now that we have more components converted, there is no need for the format to be different.

Fixes #24091

@mui-pr-bot
Copy link

mui-pr-bot commented Jan 18, 2021

No bundle size changes

Generated by 🚫 dangerJS against d9d8454

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 27, 2021

we still haven't released the StyledEngineProvider

Blocker resolved 👌

@mnajdova mnajdova changed the title [WIP][examples] Update examples to use StyledEngineProvider [examples] Update examples to use StyledEngineProvider Jan 28, 2021
@mnajdova
Copy link
Member Author

mnajdova commented Jan 28, 2021

I've run all changed examples, everything seems to work fine except the create-react-app-with-styled-components example. It throws with this error:

image

Going to create PR for it.


Edited: Merged PR #24671 for fixing it.

@mnajdova mnajdova marked this pull request as ready for review January 28, 2021 14:05
@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation. label Jan 28, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 28, 2021

I have pushed a commit d9d8454 with:

  1. A reminder so developers that copy & paste the examples know that they can remove the injectFirst option at some point. It's also relevant for us in case we forget once JSS is gone.
  2. Move away from makeStyles
  3. Remove the StrictMode wrapper in one of the examples. I believe we are waiting on Warnings in strict mode #13394 to add this block cc @eps1lon.

@oliviertassinari
Copy link
Member

on all examples except the nextjs, nextjs-with-typescript and ssr

@mnajdova I'm not sure to follow the rationale. Is it because it's too time-consuming to reproduce what docs/_app.js is doing? Or something else?

@mnajdova
Copy link
Member Author

@mnajdova I'm not sure to follow the rationale. Is it because it's too time-consuming to reproduce what docs/_app.js is doing? Or something else?

These examples have custom emotion cache, so this approach is not applicable. They need to just add the prepend option to the custom cache. The custom cache is necessary as the same instance needs to be used between the client and the server.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 28, 2021

These examples have custom emotion cache, so this approach is not applicable. They need to just add the prepend option to the custom cache. The custom cache is necessary as the same instance needs to be used between the client and the server.

Ohh I see, the problem is already fixed :). So this is dead code? it's for styled-components

https://github.com/mui-org/material-ui/blob/2c42921936f2c9e965f53e8e2a391210930b41d4/docs/pages/_app.js#L323

@mnajdova
Copy link
Member Author

mnajdova commented Jan 28, 2021

Ohh I see, the problem is already fixed :). So this is dead code?

Ah yes, we can drop it. Good catch, I forgot we have the same on the docs :)


Edited: You are right, it's still required for styled-components :)

@mnajdova mnajdova merged commit 3e2b632 into mui:next Jan 28, 2021
natac13 pushed a commit to natac13/material-ui that referenced this pull request Jan 30, 2021
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.

[v5] Inconsistent CSS order between gatsby develop and gatsby build

3 participants