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.

Sign up for GitHub

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

Jump to bottom

[docs] Migration to pigment and NextJS integration docs are conflicting #43548

Closed
abriginets opened this issue Aug 31, 2024 · 3 comments · Fixed by #43631
Closed

[docs] Migration to pigment and NextJS integration docs are conflicting #43548

abriginets opened this issue Aug 31, 2024 · 3 comments · Fixed by #43631
Assignees
@siriwatknp
Labels
docs Improvements or additions to the documentation package: pigment-css Specific to @pigment-css/* support: docs-feedback Feedback from documentation page

Comments

@abriginets
Copy link
Contributor

abriginets commented Aug 31, 2024 •
edited by github-actions bot
Loading

Related page

https://mui.com/material-ui/migration/migrating-to-pigment-css/#configuring-the-theme

Kind of issue

Missing information

Issue description

NextJS integration documentation tells us to create a theme and override Typography's fontFamily with it
[https://mui.com/material-ui/migration/migrating-to-pigment-css/#configuring-the-theme]

const theme = createTheme({
  typography: {
    fontFamily: roboto.style.fontFamily,
  },
});

While in Pigment migration docs it doesn't mention the need of changing it to be like this:

 import type { Metadata } from 'next';
 import { Inter } from 'next/font/google';

+import '@mui/material-pigment-css/styles.css';

 export default function RootLayout(props) {
   return (
     <html lang="en">
       <body className={`${inter.className}`}>
       {/* this one here ^^^^^^^^^^^^^^^^^^ */}
         {props.children}
       </body>
     </html>
   );
 }

I was under impression that the first one was the only way of combining next/font and MUI, so I went by pigment migration docs and imported Roboto from next/font directly into next.config.mjs:

export const roboto = Roboto({
  weight: ['300', '400', '500', '700', '900'],
  display: 'swap',
  subsets: ['latin', 'latin-ext', 'cyrillic', 'cyrillic-ext'],
});

/**
 * @type {import('@pigment-css/nextjs-plugin').PigmentOptions}
 */
const pigmentConfig = {
  transformLibraries: ['@mui/material'],
  theme: createTheme({
    typography: {
      fontFamily: roboto.style.fontFamily,
    },

Obviously it didn't work

Error [ERR_UNSUPPORTED_DIR_IMPORT]: Directory import '/home/goodwin/flaut/flaut-nextjs/node_modules/next/font/google' is not supported resolving ES modules imported from /home/goodwin/flaut/flaut-nextjs/next.config.mjs
Did you mean to import "next/font/google/index.js"?
    at finalizeResolution (node:internal/modules/esm/resolve:259:11)
    at moduleResolve (node:internal/modules/esm/resolve:933:10)
    at defaultResolve (node:internal/modules/esm/resolve:1157:11)
    at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:390:12)
    at ModuleLoader.resolve (node:internal/modules/esm/loader:359:25)
    at ModuleLoader.getModuleJob (node:internal/modules/esm/loader:234:38)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:87:39)
    at link (node:internal/modules/esm/module_job:86:36) {
  code: 'ERR_UNSUPPORTED_DIR_IMPORT',
  url: 'file:///home/goodwin/flaut/flaut-nextjs/node_modules/next/font/google'
}

So here's my point. MUI doesn't explicitly tells you which way is preferable and doesn't force you into any of the possible solutions though one of them works with Pigment and the other one works by default. Is there a difference and how would it affect our apps if there is. I believe it would be better if docs were consistent and advised users to use next/font via passing class name into body tag. Or if that's only possible with Pigment, then Pigment migration docs should also highlight this change in the code snippet. Thank you.

Context

No response

Search keywords: pigment, nextjs
@abriginets abriginets added status: waiting for maintainer These issues haven't been looked at yet by a maintainer support: docs-feedback Feedback from documentation page labels Aug 31, 2024
@zannager zannager added docs Improvements or additions to the documentation package: pigment-css Specific to @pigment-css/* labels Sep 2, 2024
@DiegoAndai DiegoAndai moved this to Backlog in Material UI Sep 4, 2024
@DiegoAndai DiegoAndai moved this from Backlog to In progress in Material UI Sep 5, 2024
@siriwatknp siriwatknp mentioned this issue Sep 6, 2024
Merged
1 task
@DiegoAndai
Copy link
Member

DiegoAndai commented Sep 6, 2024 •
edited
Loading

Hey, @abriginets! Thanks for the report. We will add a section to the Pigment CSS migration guide to cover this: https://github.com/mui/material-ui/pull/43631/files#diff-603580bb6644e16532b7ca1d7356ab1aa73aa8f9fa68a13664a6406b792db4e5

If you have some time, may I ask you to try this and confirm that is solves your issue? Thanks in advance.

@DiegoAndai DiegoAndai removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 6, 2024
@abriginets
Copy link
Contributor Author

Hello, @DiegoAndai. I tried following steps from updated guide and everything seems to be easy and straighforward, no errors in the terminal now.

@github-project-automation github-project-automation bot moved this from In progress to Done in Material UI Sep 7, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 7, 2024

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

Note

We value your feedback @abriginets! How was your experience with our support team?
If you could spare a moment, we'd love to hear your thoughts in this brief Support Satisfaction survey. Your insights help us improve!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@siriwatknp siriwatknp

Labels
docs Improvements or additions to the documentation package: pigment-css Specific to @pigment-css/* support: docs-feedback Feedback from documentation page
Projects
Material UI
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[docs] Add Next.js font optimization section to Pigment CSS migration siriwatknp/material-ui
4 participants
@DiegoAndai @abriginets @siriwatknp @zannager