-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[core] Prepend "use-client" directive + add docs and examples for using MUI libraries with Next.js App Router #37656
Conversation
5b062ba
to
23df1b1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a great start! I would recommend including the @mui/material-next
and @mui/system
packages too. Would be interesting to investigate if we have a component or hook that support RSC, so that we can add logic in the script to skip some files.
5183455
to
c3202bc
Compare
b5f6d4a
to
5055d4d
Compare
9c8986b
to
bd3a9bd
Compare
8ed50fc
to
3ddadbd
Compare
Updated the packages covered here! I will look into enhancing the script in a separate PR - while testing the changes so far I found that some functions (e.g. our |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs review here. This was a massive undertaking! Thanks for owning it @mj12albert ! Mostly copy edits here but also some bigger picture questions scattered around. I'll circle back to the actual examples later.
docs/data/material/getting-started/installation/installation.md
Outdated
Show resolved
Hide resolved
docs/data/material/getting-started/installation/installation.md
Outdated
Show resolved
Hide resolved
docs/data/material/getting-started/installation/installation.md
Outdated
Show resolved
Hide resolved
docs/data/material/guides/next-js-app-router/next-js-app-router.md
Outdated
Show resolved
Hide resolved
docs/data/material/guides/next-js-app-router/next-js-app-router.md
Outdated
Show resolved
Hide resolved
docs/data/material/guides/next-js-app-router/next-js-app-router.md
Outdated
Show resolved
Hide resolved
docs/data/material/guides/next-js-app-router/next-js-app-router.md
Outdated
Show resolved
Hide resolved
docs/data/material/guides/next-js-app-router/next-js-app-router.md
Outdated
Show resolved
Hide resolved
docs/data/material/guides/next-js-app-router/next-js-app-router.md
Outdated
Show resolved
Hide resolved
c86e897
to
6be0964
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Great work @mj12albert
display: 'flex', | ||
}} | ||
> | ||
<Box> | ||
<Alert severity="info" sx={{ mt: 2, mb: 5 }}> | ||
<AlertTitle>Hello 👋</AlertTitle> | ||
This app uses the Next.js App Router and Material UI v5. | ||
</Alert> | ||
|
||
<Grid container rowSpacing={3} columnSpacing={3}> | ||
<Grid xs={6}> | ||
<MediaCard | ||
heading="CMYK" | ||
text="The CMYK color model (also known as process color, or four color) is a subtractive color model, based on the CMY color model, used in color printing, and is also used to describe the printing process itself." | ||
/> | ||
</Grid> | ||
<Grid xs={6}> | ||
<MediaCard | ||
heading="HSL and HSV" | ||
text="HSL (for hue, saturation, lightness) and HSV (for hue, saturation, value; also known as HSB, for hue, saturation, brightness) are alternative representations of the RGB color model, designed in the 1970s by computer graphics researchers." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagine this change was helpful to test the whole PR end-to-end but examples are not templates. Could we revert these changes? Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to circle back to a conversation we had a couple months ago about standardizing the layout for boilerplate examples. cc @danilo-leal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Next.js approach makes a lot of sense for me, once the setup works, they link you a template https://vercel.com/templates?framework=next.js.
@samuelsycamore I believe we already have a standard layout, do you mean improving it and enforcing it to all the examples?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep that's the idea, standardize something a little more neat and polished like the pretty startup pages used by Vercel, Vite, etc. (Mainly just about reinforcing our branding)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding to the list ⎯ I think this adds tons of value! 👍 Also, having a global "Templates" page, complementary to the individual ones found in each product's docs, also looks like a good idea!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closes 37503
This PR adds the "use-client" directive to components/hooks exported from core libraries, and a CI step to check it.
Here's a basic demo with some
@mui/material
components with (using default imports) working in Next.js 13's app dir,ThemeProvider
is not used, no (additional)use client
directives are needed inpage
orlayout
files.Here's the same one but using named imports – turns out
'use client'
is needed on all theindex.js|ts
to make this work.Summary of other changes:
use client
, but based on the CDN example I think it shouldn't matter (there is no server environment)@mui/icons-material
build script needs to be run and changes committed, but I will do it in a separate PR (it results in 20k+ changed files and will make the GitHub UI lag like no tomorrow)To try out the new example repos, replace the
@mui
packages frompackage.json
with the latest version built by CI: