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

Use a newer version of the code as link examples. #27440

Closed
wants to merge 1 commit into from

Conversation

jackbravo
Copy link

@jackbravo jackbravo commented Jul 25, 2021

The current example points to an old version of the code. Updating the link to one using material-ui next.

The current example points to an old version of the code. Updating the link to one using material-ui next.
@mui-pr-bot
Copy link

mui-pr-bot commented Jul 25, 2021

No bundle size changes (experimental)

Generated by 🚫 dangerJS against c468dbe

@oliviertassinari
Copy link
Member

No, this is for emotion, not JSS

@oliviertassinari oliviertassinari added the package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. label Jul 26, 2021
@jackbravo
Copy link
Author

@oliviertassinari , I think it is also about the new version, because, for example, MUI 5.x fails on this import:

import { ServerStyleSheets } from '@material-ui/core/styles';

@oliviertassinari
Copy link
Member

@jackbravo
Copy link
Author

😱 oh I see, sorry. But then the link is pointing to an old solution that needs to be updated, but the updated version on the same repo (that I'm pointing at) contains unrelated new emotion goodies that are not relevant for the minimal things needed for a working NextJS SSR project.

This would still need fixing, right?

@oliviertassinari
Copy link
Member

I don't understand the problem you are facing. Please explain which version you are using, which user path you took to land on a problem. There are TWO versions, with TWO different documentation.

@jackbravo
Copy link
Author

jackbravo commented Jul 27, 2021

I'm using MUI 5.x, reading the 5.x docs, following the CSS advanced guide for react which says:

You need to have a custom pages/_document.js, then copy this logic

That link is using 4.x code. If I go to the master branch on that repo, it uses 5.x but with all the emotion logic you mentioned is not needed.

The next paragraph says:

Refer to this example project for an up-to-date usage example.

And it points to the new version of the code I'm using on the PR.

@oliviertassinari
Copy link
Member

@jackbravo Ok awesome. Does the change of @mnajdova in #27466 help? This whole section about styles is deprecated.

@jackbravo
Copy link
Author

Yes it does! I didn't know all this was deprecated :-p. And even emotion/server SSR solution is now deprecated for emotion 10. Seems like the next.js example will need to be updated since it seems to be using emotion/server.

Thanks a lot!

@jackbravo
Copy link
Author

I mean update the example to be more in line with vercel/next.js#17651 and vercel/next.js#17626

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 28, 2021

@jackbravo We have just updated the Next.js examples, they are up to date and correct, what you can see on Next.js's side is wrong.

@jackbravo
Copy link
Author

jackbravo commented Jul 28, 2021

The example seems to be using @material-ui/core/styles which is deprecated, right? Here.

And it is using @emotion/server here, which also seems not recommended now according to their docs.

This only applies if you’re using vanilla Emotion or a version of Emotion prior to v10. For v10 and above, SSR just works in Next.js.

And also @emotion/cache says is not needed for most cases since it is used with sensible defaults, and only needed if:

  • Using emotion in embedded contexts such as an <iframe/>
  • Setting a nonce on any <style/> tag emotion creates for security purposes
  • Using emotion with a developer defined <style/> tag
  • Using emotion with custom Stylis plugins

Which doesn't seem to be the case. But I'm not sure.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 28, 2021

@jackbravo I'm sorry, I'm unsubscribing.

  1. @material-ui/core/styles is no deprecated, @material-ui/styles will
  2. Nesting styles tag in the body like emotion encourages is not right Improve integration with Next.js #26561 (comment).

@mnajdova
Copy link
Member

The example seems to be using @material-ui/core/styles which is deprecated, right? Here.

createTheme is not deprecated.

And it is using @emotion/server here, which also seems not recommended now according to their docs.

We are following this guide - https://emotion.sh/docs/ssr#on-server
This only applies if you’re using vanilla Emotion or a version of Emotion prior to v10. For v10 and above, SSR just works in Next.js.

@jackbravo
Copy link
Author

Right @mnajdova , on that page it says that SSR just works and you don't need to use emotion/server.

But I do see that this is part of an already existing thread! My apologies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants