Skip to content

[examples] Update examples to use StyledEngineProvider#24489

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

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

Conversation

@mnajdova

@mnajdova mnajdova commented Jan 18, 2021

Copy link
Copy Markdown
Member

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

mui-pr-bot commented Jan 18, 2021

Copy link
Copy Markdown

No bundle size changes

Generated by 🚫 dangerJS against d9d8454

@oliviertassinari

oliviertassinari commented Jan 27, 2021

Copy link
Copy Markdown
Member

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

mnajdova commented Jan 28, 2021

Copy link
Copy Markdown
Member Author

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

oliviertassinari commented Jan 28, 2021

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
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.

Comment thread examples/cdn/index.html
@oliviertassinari

oliviertassinari commented Jan 28, 2021

Copy link
Copy Markdown
Member

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

mnajdova commented Jan 28, 2021

Copy link
Copy Markdown
Member Author

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