mui / pigment-css

Pigment CSS is a zero-runtime CSS-in-JS library that extracts the colocated styles to their own CSS files at build time.
MIT License
592 stars 31 forks source link

[bug] `extendTheme` doesn't work w/ `pigment` #170

Open o-alexandrov opened 2 months ago

o-alexandrov commented 2 months ago

Steps to reproduce

Link to live example

Steps:

  1. clone repo & install deps npm ci
  2. run npm run start
  3. see the theme src/theme.ts has no effect on mui.Fab

Current behavior

extendTheme doesn't change the styles of the mui.Fab

Expected behavior

extendTheme affects mui.Fab

Context

Your environment

npx @mui/envinfo ``` "@pigment-css/react": "0.0.16", "@pigment-css/vite-plugin": "0.0.16", System: OS: macOS 14.5 Binaries: Node: 22.1.0 - /opt/homebrew/bin/node npm: 10.7.0 - /opt/homebrew/bin/npm pnpm: 9.0.2 - /opt/homebrew/bin/pnpm Browsers: Chrome: 126.0.6478.127 Edge: 112.0.1722.39 Safari: 17.5 npmPackages: @emotion/react: 11.11.4 @emotion/styled: 11.11.5 @mui/base: 5.0.0-beta.51 @mui/core-downloads-tracker: 6.0.0-dev.240424162023-9968b4889d @mui/material: 6.0.0-alpha.13 => 6.0.0-alpha.13 @mui/private-theming: 6.0.0-dev.20240529-082515-213b5e33ab @mui/styled-engine: 6.0.0-dev.20240529-082515-213b5e33ab @mui/system: 6.0.0-dev.240424162023-9968b4889d @mui/types: 7.2.14 @mui/utils: 6.0.0-dev.20240529-082515-213b5e33ab @types/react: 18.3.3 => 18.3.3 react: 18.2.0 => 18.2.0 react-dom: 18.2.0 => 18.2.0 typescript: 5.5.2 => 5.5.2 ```

Search keywords: pigment, extendTheme

Search keywords:

o-alexandrov commented 2 months ago

Thank you everyone for your great progress on PigmentCSS. @siriwatknp pinging you as the repro example might be useful

aarongarciah commented 2 months ago

@o-alexandrov can you provide a link to a live example? e.g. Codesandbox

o-alexandrov commented 2 months ago

@aarongarciah please refer to the attached GitHub repo at the top of the description

aarongarciah commented 2 months ago

@o-alexandrov I saw it, but the link text says "Link to live example", but that's not the case. Whenever possible, we ask for a live example, as stated in the steps when opening a bug report:

Screenshot 2024-07-01 at 15 42 04

Is there a reason why you can't provide a live example?

DavidGregory084 commented 2 months ago

https://stackblitz.com/github/o-alexandrov/mui-pigment-extend-theme?file=index.html

aarongarciah commented 2 months ago

@siriwatknp pinging you in case you can have a look.

siriwatknp commented 2 months ago

Vite has deps optimization that does not work with Pigment CSS

This is the vite config that work so far:

import { defineConfig } from 'vite'
import react from '@vitejs/plugin-react'
import { pigment } from '@pigment-css/vite-plugin'
import { extendTheme, stringifyTheme } from '@mui/material/styles'

const theme = extendTheme();
// @ts-expect-error ignore
theme.toRuntimeSource = stringifyTheme;

// https://vitejs.dev/config/
export default defineConfig({
  optimizeDeps: {
    include: [
      "prop-types",
      "react-is",
      "hoist-non-react-statics",
      "react",
      "react-dom",
      '@emotion/react',
      '@emotion/styled',
    ],
    exclude: ["@mui/material"],
  },
  plugins: [
    react(),
    pigment({
      theme,
      transformLibraries: ["@pigment-css/react", "@mui/material"],
    }),
  ],
})

Note: this is a workaround to get the app working, need mui/material-ui#169 to be released.

o-alexandrov commented 1 month ago

@siriwatknp @aarongarciah Please reopen the issue, as it still doesn't work using the latest versions of pigment and @mui/material and following exactly the Migration Guide.

Updated the repro. Related issue https://github.com/mui/pigment-css/issues/180

There are issues:

  1. A developer still must pass @mui/material to transformLibraries
    • w/o this option, root styleOverrides from theme.ts (in the provided repro) file are not applied
  2. extended Fab styles overrides are not applied even if you pass transformLibraries
  3. vite fails to start for the first cold start
    • to test, run rm -rf node_modules/.vite and then npm run start
aarongarciah commented 1 month ago

@o-alexandrov

  1. A developer still must pass @mui/material to transformLibraries

True, I just reproduced it.

  1. extended Fab styles overrides are not applied even if you pass transformLibraries

This is not a bug. extended is not a slot, it's a variant. The only slot in Fab is root. You can override styles based on props (like variant) like this:

 const themeRaw = extendTheme({
   components: {
     MuiFab: {
       styleOverrides: {
-        extended: {
-          borderRadius: 12,
-        },
         root: {
           background: `red !important`,
+          variants:[{
+            props: { variant: 'extended' },
+            style: { borderRadius: 12 }
+          }]
         },
       },
     },
   },
 });
  1. vite fails to start for the first cold start

True, I just reproduced it.

@siriwatknp can you have a look?

o-alexandrov commented 1 month ago

@aarongarciah cc @siriwatknp Thank you for confirming and describing point 2 in more detail. The issue in point 2 is: incorrect typings then, that allow you to pass extended prop and doesn't enforce you to migrate to the new way of declaring variants. (see screenshot).

Screenshot 2024-07-23 at 8 49 56 AM
rhoiyds commented 2 weeks ago

I'm beginning to get this exact error: Details posted over on https://github.com/mui/pigment-css/issues/211