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

Port classic components to PigmentCSS? #43471

Closed
Mike-Ro opened this issue Aug 27, 2024 · 18 comments
Closed

Port classic components to PigmentCSS? #43471

Mike-Ro opened this issue Aug 27, 2024 · 18 comments
Assignees
Labels
package: pigment-css Specific to @pigment-css/* support: question Community support but can be turned into an improvement

Comments

@Mike-Ro
Copy link

Mike-Ro commented Aug 27, 2024

Hey everyone!

Great to see v6!

Just wondering, are there any plans to port classic components like Button to PigmentCSS? If so, which version will it be for - classic MUI or JoyUI?

Thanks!

Search keywords:

@github-actions github-actions bot added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 27, 2024
@mnajdova
Copy link
Member

@Mike-Ro what do you mean with classic components? Do you mean supporting Pigment CSS in Material UI? If this is the case, you can follow the https://mui.com/material-ui/migration/migrating-to-pigment-css/ guide to enable Pigment CSS in your project - enabling it means all Material UI components would use it instead of Emotion.

@mnajdova mnajdova added support: question Community support but can be turned into an improvement status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 27, 2024
@Mike-Ro
Copy link
Author

Mike-Ro commented Aug 28, 2024

@mnajdova I’m not sure if I’m using PigmentCSS correctly. Here’s what I’ve done — I'll show you with an example:

app/page.js:

import {Button} from '@mui/material';
import {css} from '@mui/material-pigment-css';

export default function page() {
  const styledTestPigmentCss = css({
    border: '1px solid black',
    textTransform: 'uppercase',
  });

  return (
    <>
      <Button color="primary">Test Button</Button>
      <div className={styledTestPigmentCss}>Test PigmentCSS</div>
    </>
  );
}

When I run npm run build, an ``out directory is created in the root of the project, and it contains a single index.html page with two elements:

div "Test PigmentCSS" – a separate CSS file with styles is created for this:
4589344574

Button "Test Button", there’s no separate CSS file created for it, and the styles are applied inline in the header with a <style data-emotion="css 6v2v4c">:
458932346

I tried disabling transformLibraries: ['@mui/material'], but it doesn’t seem to change anything... Am I doing something wrong?

.
.
.
.
.
.
Project config:

package.json:

{
  "name": "test",
  "version": "0.1.0",
  "private": true,
  "scripts": {
    "dev": "next dev",
    "build": "next build",
    "start": "next start",
    "lint": "next lint"
  },
  "dependencies": {
    "@mui/material": "^6.0.0",
    "@mui/material-pigment-css": "^6.0.0",
    "next": "^14.2.6",
    "react": "^18.3.1",
    "react-dom": "^18.3.1"
  },
  "devDependencies": {
    "@next/bundle-analyzer": "^14.2.5",
    "@pigment-css/nextjs-plugin": "^0.0.20"
  }
}

next.config.mjs:

import {withPigment} from '@pigment-css/nextjs-plugin';

/** @type {import('next').NextConfig} */
const nextConfig = {
  output: 'export',
  images: {
    unoptimized: true,
  },
  trailingSlash: false,
};

/**
 * @type {import('@pigment-css/nextjs-plugin').PigmentOptions}
 */
const pigmentConfig = {
  transformLibraries: ['@mui/material'],
};

export default withPigment(nextConfig, pigmentConfig);

app/layout.js:

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

export default function RootLayout({children}) {
  return (
    <html lang="en">
    <body>
    {children}
    </body>
    </html>
  );
}

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Aug 28, 2024
@mnajdova
Copy link
Member

Your config looks correct (you should have the transformLibraries: ['@mui/material'],). Can you try moving the css call outside of the render function for the page, e.g. like this:

import {Button} from '@mui/material';
import {css} from '@mui/material-pigment-css';

+  const styledTestPigmentCss = css({
+    border: '1px solid black',
+    textTransform: 'uppercase',
+  });
 
export default function page() {
-  const styledTestPigmentCss = css({
-    border: '1px solid black',
-    textTransform: 'uppercase',
-  });

  return (
    <>
      <Button color="primary">Test Button</Button>
      <div className={styledTestPigmentCss}>Test PigmentCSS</div>
    </>
  );
}

I would recommend checking https://github.com/mui/material-ui/tree/master/examples/material-ui-pigment-css-nextjs-ts on how to set up the project (just use the `^6.0.0 version on all @mui/* packages). If you can't make it work please create a github repo with the project and I can check it.

@mnajdova mnajdova self-assigned this Aug 28, 2024
@Mike-Ro
Copy link
Author

Mike-Ro commented Aug 28, 2024

@mnajdova I tried your example, but I’m still running into the same issue :(

I created a test repo here: https://github.com/Mike-Ro/test-muiv6 and tried another approach:

import {styled} from '@mui/material-pigment-css';

const StyledButton = styled(Button)(() => ({
  color: 'red',
}));

But I'm getting the same result - the red color is being moved to a separate CSS file, while the main button styles are still being applied inline in the header.

34782346

@mnajdova
Copy link
Member

mnajdova commented Aug 28, 2024

Looks like the Button component still uses emotion although I added the transformLibraries: ['@mui/material'], with the default Mateiral UI theme. It seems to be something with the next.js plugin, as for vite we have an app where this works. cc @siriwatknp have you tested this scenario with next.js so far?

@Mike-Ro
Copy link
Author

Mike-Ro commented Aug 28, 2024

@mnajdova

Looks like the Button component still uses emotion although I added the transformLibraries: ['@mui/material'], with the default Mateiral UI theme.

Got it, I thought this was normal behavior.

It seems to be something with the next.js config

Are you referring to my next.js config? It's almost the default setup.

@mnajdova
Copy link
Member

mnajdova commented Aug 28, 2024

I meant the next.js plugin sorry, let me edit the comment! Your setup seems good, the same setup works in vite (using vite's plugin), so I assume is something related to the next.js plugin.

@mnajdova
Copy link
Member

@siriwatknp even the example we have added in #43065 it's not working as expected. The Material UI components still use emotion, check the images:

Screenshot 2024-08-28 at 13 58 41 Screenshot 2024-08-28 at 13 59 00

Also, if I disable JS the Pigment CSS related styles are not being injected, which means SSR is broken. Have you looked into this so far?

@siriwatknp
Copy link
Member

siriwatknp commented Aug 28, 2024

@siriwatknp even the example we have added in #43065 it's not working as expected. The Material UI components still use emotion, check the images:

Something's wrong with the build process on master branch (probably due to f4a7047). I changed to the latest beta and it works.

Here is the Material UI Accordion after build:

v6 RC:

import { styled } from '../zero-styled';

v6 latest:

import { styled } from '../zero-styled/index.js';

@siriwatknp
Copy link
Member

@siriwatknp even the example we have added in #43065 it's not working as expected. The Material UI components still use emotion, check the images:

Screenshot 2024-08-28 at 13 58 41 Screenshot 2024-08-28 at 13 59 00
Also, if I disable JS the Pigment CSS related styles are not being injected, which means SSR is broken. Have you looked into this so far?

✅ Solution

Update Pigment CSS plugin to at least 0.0.21:

Next.js

"@pigment-css/nextjs-plugin": "0.0.21"

Vite

"@pigment-css/vite-plugin": "0.0.21"

@siriwatknp siriwatknp removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 29, 2024
@mnajdova
Copy link
Member

@Mike-Ro let us know if updating the plugin to 0.0.21 solves the issue for you, so we can close it :) Thanks @siriwatknp for looking into this

@Mike-Ro
Copy link
Author

Mike-Ro commented Aug 29, 2024

@mnajdova @siriwatknp updated @pigment-css/nextjs-plugin to 0.0.21 in the test project (https://github.com/Mike-Ro/test-muiv6), but nothing changed :(

45634367

@mnajdova
Copy link
Member

@siriwatknp https://github.com/mui/pigment-css/pull/214/files#diff-be3ad8d071bd1d35260116abddeb06f9713e0c895d5008a3734c109855a01017 has changes to the @pigment-css/react package too, but we haven't migrated the @mui/material-pigment-css package to include this change. Could this be creating issues?

@siriwatknp
Copy link
Member

@siriwatknp https://github.com/mui/pigment-css/pull/214/files#diff-be3ad8d071bd1d35260116abddeb06f9713e0c895d5008a3734c109855a01017 has changes to the @pigment-css/react package too, but we haven't migrated the @mui/material-pigment-css package to include this change. Could this be creating issues?

I don't think so. I tested with the Material UI Pigment CSS example (Next.js) by upgrading only the @pigment-css/nextjs-plugin and it works.

@mnajdova
Copy link
Member

@Mike-Ro try this change in your repo, it should fix the issue: https://github.com/Mike-Ro/test-muiv6/pull/1/files

@mnajdova
Copy link
Member

I am closing this, thanks @siriwatknp!

Copy link

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 @Mike-Ro! 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!

@Mike-Ro
Copy link
Author

Mike-Ro commented Aug 29, 2024

@Mike-Roпопробуйте это изменение в вашем репозитории, это должно исправить проблему: https://github.com/Mike-Ro/test-muiv6/pull/1/files

It works, thanks!

@DiegoAndai DiegoAndai moved this to Done in Material UI Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: pigment-css Specific to @pigment-css/* support: question Community support but can be turned into an improvement
Projects
Status: Done
Development

No branches or pull requests

3 participants