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

[RFC] Zero-runtime CSS-in-JS implementation #38137

Open
brijeshb42 opened this issue Jul 24, 2023 · 138 comments
Open

[RFC] Zero-runtime CSS-in-JS implementation #38137

brijeshb42 opened this issue Jul 24, 2023 · 138 comments
Assignees
Labels
package: pigment-css Specific to @pigment-css/* package: system Specific to @mui/system performance RFC Request For Comments

Comments

@brijeshb42
Copy link
Contributor

brijeshb42 commented Jul 24, 2023

What's the problem? 🤔

This RFC is a proposal for implementing a zero-runtime CSS-in-JS solution to be used in a future major version of Material UI and Joy UI.

TLDR: We are planning to develop a custom implementation of a zero-runtime CSS-in-JS library with ideas from Linaria and Compiled.

With the rising popularity of React Server Components (RSCs), it’s important that we support this new pattern for all components that are compatible. This mainly applies to layout components such as Box, Typography, etc., as they are mostly structural and the only blocker for RSC compatibility is the use of Emotion.

Another aspect is the use of themes. Currently, they need to be passed through a Provider component (especially if an application is using multiple themes) which uses React Context. RSCs do not support states/contexts.

In the last major version, we moved the styling solution to Emotion for more performant dynamic styles. Since then, Internet Explorer has been deprecated, enabling us to go all in on CSS Variables. We already use this with an optional provider (CSS theme variables - MUI System).

What are the requirements? ❓

  • Minimal runtime for peak performance and negligible JS bundle size as far as the runtime is concerned.
  • Supporting RSC as part of the styling solution means no reliance on APIs unavailable to server components (React Context).
  • Keep the same DX. You should still be able to use the sx prop along with container-specific props like <Box marginTop={1} />etc.
  • It should be possible to swap out the underlying CSS-in-JS pre-processor. We have already explored using emotion as well as stitches, as mentioned below.
  • Source map support. Clicking on the class name in the browser DevTools should take you to the style definitions in the JS/TS files.
  • Minimal breaking changes for easier migration.

What are our options? 💡

We went through some of the existing zero-runtime solutions to see if they satisfy the above requirements.

  1. vanilla-extract - This ticks most of the boxes, especially when used along with libraries like dessert-box. But its limitation to only be able to declare styles in a .css.ts file felt like a negative point in DX.
  2. Compiled - Compiled is a CSS-in-JS library that tries to follow the same API as Emotion which seems like a win, but it has some cons:
    • Theming is not supported out of the box, and there’s no way to declare global styles.
    • Atomic by default. No option to switch between atomic mode and normal CSS mode.
  3. Linaria - Linaria in its default form only supports CSS declaration in tagged template literals. This, along with no theming support as well as no way to support the sx prop led us to pass on Linaria.
  4. PandaCSS - PandaCSS supports all the things that we require: a styled function, Box props, and an equivalent of the sx prop. The major drawback, however, is that this is a PostCSS plugin, which means that it does not modify the source code in place, so you still end up with a not-so-small runtime (generated using panda codegen) depending on the number of features you are using. Although we can’t directly use PandaCSS, we did find that it uses some cool libraries, such as ts-morph and ts-evaluate to parse and evaluate the CSS in its extractor package.
  5. UnoCSS - Probably the fastest since it does not do AST parsing and code modification. It only generates the final CSS file. Using this would probably be the most drastic and would also introduce the most breaking changes since it’s an atomic CSS generation engine. We can’t have the same styled() API that we know and love. This would be the least preferred option for Material UI, especially given the way our components have been authored so far.

Although we initially passed on Linaria, on further code inspection, it came out as a probable winner because of its concept of external tag processors. If we were to provide our own tag processors, we would be able to support CSS object syntax as well as use any runtime CSS-in-JS library to generate the actual CSS. So we explored further and came up with two implementations:

  1. emotion - The CSS-in-JS engine used to generate the CSS. This Next.js app router example is a cool demo showcasing multiple themes with server actions.
  2. no-stitches - Supports the styled API from Stitches. See this discussion for the final result of the exploration.

The main blocker for using Linaria is that it does not directly parse the JSX props that we absolutely need for minimal breaking changes. That meant no direct CSS props like <Box marginTop={1} /> or sx props unless we converted it to be something like <Component sx={sx({ color: 'red', marginTop: 1 })} />. (Note the use of an sx function as well.) This would enable us to transform this to <Component sx="random-class" /> at build-time, at the expense of a slightly degraded DX.

Proposed solution 🟢

So far, we have arrived at the conclusion that a combination of compiled and linaria should allow us to replace styled calls as well as the sx and other props on components at build time. So we’ll probably derive ideas from both libraries and combine them to produce a combination of packages to extract AST nodes and generate the final CSS per file. We’ll also provide a way to configure prominent build tools (notably Next.js and Vite initially) to support it.

Theming

Instead of being part of the runtime, themes will move to the config declaration and will be passed to the styled or css function calls. We’ll be able to support the same theme structure that you know created using createTheme from @mui/material.

To access theme(s) in your code, you can follow the callback signature of the styled API or the sx prop:

const Component = styled('div')(({ theme }) => ({
  color: theme.palette.primary.main,
  // ... rest of the styles
}))
// or
<Component sx={({ theme }) => ({ backgroundColor: theme.palette.primary... })} />

Although theme tokens’ structure and usage won’t change, one breaking change here would be with the component key. The structure would be the same, except the values will need to be serializable.

Right now, you could use something like:

const theme = createTheme({
  components: {
    // Name of the component
    MuiButtonBase: {
      defaultProps: {
        // The props to change the default for.
        disableRipple: true,
        onClick() {
          // Handle click on all the Buttons.
	}
      },
    },
  },
});

But with themes moving to build-time config, onClick won’t be able to be transferred to the Button prop as it’s not serializable. Also, a change in the styleOverrides key would be required not to use ownerState or any other prop values. Instead, you can rely on the variants key to generate and apply variant-specific styles

Before

const theme = createTheme({
  components: {
    MuiButton: {
      styleOverrides: {
        root: ({ ownerState }) => ({
          ...(ownerState.variant === 'contained' &&
            ownerState.color === 'primary' && {
              backgroundColor: '#202020',
              color: '#fff',
            }),
        }),
      },
    },
  },
});

After

const theme = createTheme({
  components: {
    MuiButton: {
      variants: [
        {
          props: { variant: 'contained', color: 'primary' },
          style: {
            backgroundColor: '#202020',
            color: '#fff'
          },
        },
      ],
    },
  },
});

Proposed API

The styled API will continue to be the same and support both CSS objects as well as tagged template literals. However, the theme object will only be available through the callback signature, instead of being imported from a local module or from @mui/material :

// Note the support for variants
const Component = styled('div')({
  color: "black",
  variants: {
    size: {
      small: {
	fontSize: '0.9rem',
	margin: 10
      },
      medium: {
        fontSize: '1rem',
	margin: 15
      },
      large: {
	fontSize: '1.2rem',
	margin: 20
      },
    }
  },
  defaultVariants: {
    size: "medium"
  }
})
// Or: 
const ColorComponent = styled('div')(({ theme }) => ({
  color: theme.palette.primary.main
});

The theme object above is passed through the bundler config. At build-time, this component would be transformed to something like that below (tentative):

const Component = styled('div')({
  className: 'generated-class-name',
  variants: {
    size: {
      small: "generated-size-small-class-name",
      medium: "generated-size-medium-class-name",
      large: "generated-size-large-class-name",
    }
  }
});
/* Generated CSS:
.generated-class-name {
  color: black;
}
.generated-size-small-class-name {
  font-size: 0.9rem;
  margin: 10px;
}
.generated-size-medium-class-name {
  font-size: 1rem;
  margin: 15px;
}
.generated-size-large-class-name {
  font-size: 1.2rem;
  margin: 20px;
}
*/

Dynamic styles that depend on the component props will be provided using CSS variables with a similar callback signature. The underlying component needs to be able to accept both className and style props:

const Component = styled('div')({
  color: (props) => props.variant === "success" ? "blue" : "red",
});

// Converts to:
const Component = styled('div')({
  className: 'some-generated-class',
  vars: ['generated-var-name']
})

// Generated CSS:
.some-generated-class {
  color: var(--generated-var-name);
}

// Bundled JS:
const fn1 = (props) => props.variant === "success" ? "blue" : "red"

<Component style={{"--random-var-name": fn1(props)}} />

Other top-level APIs would be:

  1. css to generate CSS classes outside of a component,
  2. globalCss to generate and add global styles. You could also directly use CSS files as most of the modern bundlers support it, instead of using globalCss.
  3. keyframes to generate scoped keyframe names to be used in animations.

Alternative implementation

An alternative, having no breaking changes and allowing for easy migration to the next major version of @mui/material is to have an opt-in config package, say, for example, @mui/styled-vite or @mui/styled-next. If users don’t use these packages in their bundler, then they’ll continue to use the Emotion-based Material UI that still won’t support RSC. But if they add this config to their bundler, their code will be parsed and, wherever possible, transformed at build time. Any static CSS will be extracted with reference to the CSS class names in the JS bundles. An example config change for Vite could look like this:

import { defineConfig } from "vite";
import react from "@vitejs/plugin-react";
// abstracted plugin for vite
import styledPlugin from "@mui-styled/vite";
import { createTheme } from "@mui/material/styles";

const customTheme = createTheme({
  palette: {
    primary: {
      main: '#1976d2',
    },
  },
  components: {
    MuiIcon: {
      styleOverrides: {
        root: {
          boxSizing: 'content-box',
          padding: 3,
          fontSize: '1.125rem',
        },
      },
    },
  }
  // ... other customizations that are mainly static values
});

// https://vitejs.dev/config/
export default defineConfig(({ mode }) => ({
  plugins: [
    styledPlugin({
      theme: customTheme,
      // ... other tentative configuration options
    }),
    react(),
  ]
}));

For component libraries built on top of Material UI, none of the above changes would affect how the components are authored, except for the need to make it explicit to users about their theme object (if any), and how that should be imported and passed to the bundler config as discussed above.

Known downsides of the first proposal

Material UI will no longer be a just install-and-use library: This is one of the features of Material UI right now. But with the changing landscape, we need to compromise on this. Several other component libraries follow a similar approach.
Depending on the bundler being used, you’ll need to modify the build config(next.config.js for Next.js, vite.config.ts for Vite, etc.) to support this. What we can do is provide an abstraction so that the changes you need to add to the config are minimal.

Resources and benchmarks 🔗

Playground apps -

  1. Next.js
  2. Vite

Related issue(s)

@brijeshb42 brijeshb42 added package: system Specific to @mui/system status: waiting for maintainer These issues haven't been looked at yet by a maintainer RFC Request For Comments labels Jul 24, 2023
@mnajdova mnajdova removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 24, 2023
@mwskwong
Copy link
Contributor

<Component sx={({ theme }) => ({ backgroundColor: theme.palette.primary... })} />

I'm a bit concerned about how the theme is accessed. Right now, if Component is a client component while the parent is a server component, we can't access the theme like this because functions are not serializable.

@acomanescu
Copy link

acomanescu commented Jul 24, 2023

@mwskwong We should use CSS vars. A lot faster and easier to write. We'll miss the typecheck, but we'll have to live with that.

<Component sx={{ backgroundColor: 'primary' }} />

@brijeshb42
Copy link
Contributor Author

brijeshb42 commented Jul 24, 2023

@mwskwong This is what you write in your code which will then be replaced at build time (dev or prod) with the generated css. theme will be callback argument passed by the said tool to your function. It won't matter in that case whether it's a client or a server component as far as the sx prop is concerned.

See the POC code where Home is a server component.

@JanStevens
Copy link

Nice! I wonder how nested themes would be supported 🤔.

Our use case: We heavily use nested themes where our main theme overwrites almost every component. For specific pages we have a unique look and feel where we use nested themes that partially update some components (ex font family for all h1-h4 or fully rounded buttons).

Another use case: we have a fully dark based theme except for our ecommerce pages they are completely light theme based (again with a nested light theme), but the header and footer stay in the dark theme for example 😅.

Looking forward! Regards

@astahmer
Copy link

hey, this looks amazing !

regarding Panda, I wanted to add a little more infos:

The major drawback, however, is that this is a PostCSS plugin, which means that it does not modify the source code in place, so you still end up with a not-so-small runtime (generated using panda codegen) depending on the number of features you are using.

I can understand that. You could evaluate those css calls etc at build-time I guess tho, as it was done by the Qwik people here

Although we can’t directly use PandaCSS, we did find that it uses some cool libraries, such as ts-morph and ts-evaluate to parse and evaluate the CSS in its extractor package.

you could use the extractor on its own if that helps, it has no panda-specific dependencies !

@OlegLustenko
Copy link

OlegLustenko commented Jul 24, 2023

This could be a brave idea, but what do you think about completely dropping or making opt-in CSS-IN-JS in favor of libraries like TailwindCSS or UnoCSS for styling?

It could be an ambitious, but long-term win solution and definitely a win-win decision

No matter what you will do, you will have to implement an intermediate layer.

@damassi
Copy link

damassi commented Jul 24, 2023

One question: Is CSS-in-JS support incoming for RSC on the React side? There has been some discussion around this, though I can't find where.

@mwskwong
Copy link
Contributor

mwskwong commented Jul 24, 2023

One question: Is CSS-in-JS support incoming for RSC on the React side? There has been some discussion around this, though I can't find where.

The reason why runtime CSS-in-JS (not just CSS in JS in general to be exact) has so many problems in the new architecture of React is because as its name suggests, it needs JS in runtime to function, while RSC does the exact opposite.

For this RFC, we are talking about zero runtime CSS-in-JS, so ideally, everything will be converted into plain CSS during built-time and won't have the issue we are facing. Although one thing to consider is whether it can maintain a certain degree of dynamic since we no longer have access to resources like theme during runtime.

@NicestRudeGuy
Copy link

NicestRudeGuy commented Jul 25, 2023

Why not use CSS Modules for theming ? I believe this would be simpler and easier to do. Though the dynamic styling part needs to be figured out. Maybe use good old styles object with CSSProperties from react for type safety.

We recently diteched our old DS which used context and CSS-IN-JS for theming and the new CSS Variables are way easier to do with Design tokens as well. Performant and can easily style components if required.
Can also do Module Scss if you would like.

We took alot of inspiration from MUI the way its built and the components API as well. Thanks for building this amazing Library.

@mwskwong
Copy link
Contributor

mwskwong commented Jul 25, 2023

@mwskwong We should use CSS vars. A lot faster and easier to write. We'll miss the typecheck, but we'll have to live with that.

The situation I was mentioning can also appear to the new CssVarsProvider API, which is using CSS variables. e.g.

<List size="sm" sx={{ "--List-radius": theme => theme.vars.radius.sm }} />

Such a way of accessing the radius CSS var is more scalable. And yes, I can also do "--List-radius": "var(--joy-radius-sm)" (which is what I'm doing for the sake of RSC compatibility), but I'm taking the risk of making typos.

@mwskwong This is what you write in your code which will then be replaced at build time (dev or prod) with the generated css. theme will be callback argument passed by the said tool to your function. It won't matter in that case whether it's a client or a server component as far as the sx prop is concerned.

See the POC code where Home is a server component.

Thanks @brijeshb42 on elaborate on that part, I would say that will be the best of both worlds.

@mnajdova
Copy link
Member

Another use case: we have a fully dark based theme except for our ecommerce pages they are completely light theme based (again with a nested light theme), but the header and footer stay in the dark theme for example 😅.

@JanStevens, we can support this by injecting CSS variables wherever we had previously nested ThemeProvider components. The theme structure would be the same, the usage would be the same, and only the CSS variables' values would change.

@mwskwong
Copy link
Contributor

mwskwong commented Jul 25, 2023

Another use case: we have a fully dark based theme except for our ecommerce pages they are completely light theme based (again with a nested light theme), but the header and footer stay in the dark theme for example 😅.

@JanStevens, we can support this by injecting CSS variables wherever we had previously nested ThemeProvider components. The theme structure would be the same, the usage would be the same, and only the CSS variables' values would change.

Just wondering, with the introduction of data-mui-color-scheme="dark" (or light), does nested ThemeProvider still need to be supported?

@brijeshb42
Copy link
Contributor Author

@mwskwong We might still keep the ThemeProvider component as-is for compatibility but it'll be replaced at build time with a simple div or as prop. We are still exploring the finer details.

@brijeshb42
Copy link
Contributor Author

@astahmer I did explore using @pandacss/extractor package and even had a working demo. But later I found that ts-evaluator is not very foolproof compared to how Linaria evaluates values (using node:module). That led us to ditch the whole package along with ts-morph. I feel the extraction part itself is simpler but the main part of the implementation resides in how we can evaluate the extracted AST nodes.

@SC0d3r
Copy link

SC0d3r commented Jul 25, 2023

One question: Is CSS-in-JS support incoming for RSC on the React side? There has been some discussion around this, though I can't find where.

The reason why runtime CSS-in-JS (not just CSS in JS in general to be exact) has so many problems in the new architecture of React is because as its name suggests, it needs JS in runtime to function, while RSC does the exact opposite.

For this RFC, we are talking about zero runtime CSS-in-JS, so ideally, everything will be converted into plain CSS during built-time and won't have the issue we are facing. Although one thing to consider is whether it can maintain a certain degree of dynamic since we no longer have access to resources like theme during runtime.

Having no JS after compilation will be a major upgrade for MUI (YES! less JS shipped is always a good thing 👌), I hope you guys still support sx cause without it life gonna pretty hard!

@brijeshb42
Copy link
Contributor Author

@SC0d3r Yes. We'll still have sx prop. See our exploratory POC.

@ShanonJackson
Copy link

ShanonJackson commented Jul 25, 2023

Going to just brain dump alot of thoughts about Linaria and Styled-Components after many many years of using them before going full circle back to SCSS for the last 1.5 years and never looking back.

Linaria

  • Linaria relies on build tooling much like css modules with the difference being it's got a much smaller community and is unlikely to be supported FIRST in all meta frameworks. Using custom webpack loaders will become less and less reliable over time as people explore using more performant tooling written in rust and go where extensions are required to be written in those languages respectively making support for custom build tooling more difficult.
  • Linaria and CSS modules with scss are almost the same except that regular scss compiles down to css with near zero runtime impact, Linaria compiles down to regular css but still wraps every element in a component that passes class name meaning you're rendering a wrappper component for every tag.
  • Linaria has variables, loops, functions, globally unique class names, ... etc - custom themeing via css variables.
  • SCSS (with css modules) has variables, loops, functions, globally unique class names, etc - custom themeing via css variables.
    It's only the syntax that differs between them.
  • SCSS has existed in the frontend ecosystem longer than React and will probably exist long after, hiring people who know scss is much more likely than hiring people who know Linaria, and css-in-js.
  • Linaria does have support for nested styles and nested selectors however it ruins build time performance by exponential factors because modern tooling operates on a single file basis making resolving imports to their respective values at build time very expensive as you need to traverse backwards or forwards through the import tree resulting in many passes over the same file. Almost no other loaders in webpack or extensions in babel work like this because the performance is so bad.
  • SCSS with CSS modules (css loader) will forever have first class lifetime support in all meta frameworks by default.
  • Linaria obfuscates semantic HTML from developers

Converting to/from linaria, styled-components/emotion and css modules is easier than people might think and because it's mostly based on patterns I'd say it would be possible to code-mod most of it or do an incremental approach 1 component at a time.

Variables to/from

// in
Button.styles.ts
export const Button= styled.button`
	background-color: ${(p) => p.theme.variants.secondary.backgroundColor};
`;
// out .scss
.button {
    background-color: var(--secondary-background-color);
}

Modifers to/from

// Button.styles.ts
export const Button= styled.button`
	background-color: ${(p) => p.theme.variants.secondary.backgroundColor};
	{({primary}) => {
	     return css`color: red;`
	}
`;
// out .scss
.button {
    background-color: var(--secondary-background-color);
    &--primary {
        color: red;
    }
}

// Button.tsx
return <button className={cx(styles.button, theme === "primary" && styles.buttonPrimary)}>{children}<button>;

Loops to/from - Just use scss loops.
Reusable snippets to/from - Just use mixins

=== Final Thoughts ===

This may not be completely feasible with the MUI codebase but thought I'd share my long experience with Linaria to allow you to avoid potential headaches where possible.

Our overall UI library and application architecture was much different than MUI with less configurability so while this was the best choice for us for the reasons above it may still be a good option for you.

Will leave this here just to prove I was participating in Linaria community 4+ years ago
callstack/linaria#501

@saviorhavely
Copy link

Panda-css is amazing, I did some tests when I was looking for a lib that would solve the css-in-js problem. However it is in its first releases and I want to see what else they are planning but I can say that I trust the team at Chakra-ui a lot.

@sag1v
Copy link

sag1v commented Jul 26, 2023

Everything sounds great though i do have a major concern (or a question) here.

Lets say i have a Design System library that wraps MUI, we don't have a build process other than passing our code via babel cli for both cjs and esm transpilations.

My concern/question is, Do we now forced to use a bundler or any other tool or framework to be able to convert the styles in build time?

The 2 possible solutions i see here, and i might be way off:

  1. Let the consumers of my lib pass it through a build.
  2. Somehow do it via a babel plugin, which might hurt performance (degrade DX) due to the fact we will need to parse AST.

@brijeshb42
Copy link
Contributor Author

@sag1v We've already covered this in the RFC

For component libraries built on top of Material UI, none of the above changes would affect how the components are authored, except for the need to make it explicit to users about their theme object (if any), and how that should be imported and passed to the bundler config as discussed above (if the user has included the zero runtime config in their bundler setup).

@achan-godaddy
Copy link

achan-godaddy commented Jul 26, 2023

But later I found that ts-evaluator is not very foolproof compared to how Linaria evaluates values (using node:module).

seems like everything that is pure can be compiled away down to css classes, and everything that has some form of conditional or complex js needs to either remain with some small runtime or compile down to something that looks like a classnames/clsx style. Just looking for recognized strings is basically what the tailwind compiler does to generate the classes.

// pure can be compiled down to classes
sx={{ padding: 3, color: 'red' }} => className="p-3px red"

// dynamic needs to retain some runtime
sx={{ padding: 3, color: isActive ? 'red' : 'blue' }} => className={classNames("p-3px", isActive ? 'red' : 'blue')}

So the strings/numbers become the important thing to interpret and then any complex code is given up on and left inline which leaves some runtime code but I don't know any way around it. The problem with inline code is always that it needs to be interpreted which is why vanilla extract just evals to css and leaves the dynamicism to their sprinkles js runtime.

@kylemh
Copy link

kylemh commented Jul 26, 2023

I was wondering... Instead of creating MUI's own zero-runtime CSS-in-JS solution, what about helping PandaCSS migrate from PostCSS to LightningCSS? MUI spends less time re-inventing the wheel and makes a great, existing CSS solution even better.

@brijeshb42
Copy link
Contributor Author

brijeshb42 commented Jul 27, 2023

@kylemh As explained in the RFC about panda, it's not about whether panda is using postcss or lightningcss to generate the css. Panda does not modify the original code. Which means whatever you write in your code along with how panda generates the classnames remain part of the final build and hence increase the overall bundle size together with its runtime.

@kylemh
Copy link

kylemh commented Jul 27, 2023

Interesting. Doesn't that mean their claim to zero runtime is incorrect?

Also, another question... Will the solution MUI build be integrated as part of the monorepo or as a separate repository? If somebody wanted to use the CSS-in-JS library, will the relevant APIs be code-splittable?

@danilo-leal danilo-leal changed the title [RFC][system] Zero-runtime CSS-in-JS implementation [RFC] Zero-runtime CSS-in-JS implementation May 3, 2024
@danilo-leal danilo-leal added the package: pigment-css Specific to @pigment-css/* label May 3, 2024
@hmd-ai
Copy link

hmd-ai commented Jun 4, 2024

is there a guide for using MUI v6 for SSR? The current guide seems to be based on v5 (Emotion):
https://next.mui.com/material-ui/guides/server-rendering/

@danilo-leal
Copy link
Contributor

@hmd-ai not yet, but we'll be adding Material UI docs for opting into Pigment CSS very soon (follow this issue #42523).

@hooper-hc

This comment was marked as off-topic.

@frattaro
Copy link

I'm just some random guy who happened upon the thread, but...

not a fan of sx, because it's only available on components from this library. love emotion's css prop, since it's available everywhere. Seeing your requirements, I'd suggest you do what emotion did and make your own jsx parser -- that way you could pick a syntax, abstract the underlying css lib, and it'd be available everywhere

@yesudeep
Copy link

I'm not sure whether this solution will continue to include inline styles, but inline styles will become a blocker soon as Websites start deploying strict CSP and start using nonces/hashes within these policies. Can we please ensure inline styles are no longer included within MUI?

See: #19938

@brijeshb42
Copy link
Contributor Author

@frattaro Since this is a build time solution, we have made sx props available on direct html elements and not just to components imported from the library.

@frattaro
Copy link

@brijeshb42 would custom components then need to drill sx down to the first root html element?

@LeoVS09
Copy link

LeoVS09 commented Jul 18, 2024

Honestly, I see this RFC as over engineering of existing solutions. CSS-in-JS DX not so good as it hyped. Regular scss modules much more flexible and not increase code of components. They allow to incapsulate styles from logic and make DX of reading code (which happens much more often than writing it) more pleasant.

I suggest to think again about requirement to keep CSS-in-JS. With much less efforts you can migrate all components to scss modules and keep support of sx and styled stuff.
Library will become much more faster and RCS complained, while keeping it easy to install and use, without breaking changes.

@frattaro
Copy link

@LeoVS09 that's a misapplication of architectures. components don't benefit from MVC as it results in spaghetti components (scrolling all over, switching files to find a class definition) when it's much more maintainable to keep concerns as colocated to their application as possible. Whether it be inline styles, atomic classes, css-in-js they all recognize that applying the styles directly to the component they affect is the desirable dx.

@o-alexandrov
Copy link
Contributor

o-alexandrov commented Jul 18, 2024

The most important part of CSS-in-JS for me is the ability to reuse values between styles and JS, such as CSS variables' names, some CSS values for calculations in JS.

It also results in simpler DX due to no need to learn Sass, Less, Stylus, etc.

  • in your team, you just need to make sure you know JavaScript, and CSS

The worst part of Pigment CSS is speed. For MacBook Pro M2 Pro or Max, it adds at least 5 seconds to the overhead to start a small project in Vite and ~10 seconds in a medium one. And I'm sorry for anyone who would try to run it in a big project. HMR mostly works, but sometimes (at least once a day) requires you to restart a Vite server

  • in other words, current DX is bad when you start a project due to long wait times, and okay while you work due to imperfect HMR, but hopefully, one day Pigment would rely on SWC (related issue) and obviously over time HMR edge cases would all be reported and fixed

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 19, 2024

The worst part of Pigment CSS is speed

@o-alexandrov Did you happen to measure the build time speed increase? I'm curious.

it adds at least

What's the start time if you disable Pigment CSS in dev mode? I'm trying to get a sense of the % increase. If it's 1s and its get to 11s, it's a different story than if it's 30s and gets to 40s. In the first case, it feels like a deal breaker, that I can't use this for a project that will grow to 20+ engineers because everything will get incredibly slow in dev mode as the codebase grow. In the other case, it feels like it could be better, but I can make this tradeoff.

@o-alexandrov
Copy link
Contributor

o-alexandrov commented Jul 19, 2024

@oliviertassinari in a medium size project, the hot start (after vite created optimized deps) was ~0.8 seconds, now it's ~23s, so it's about ~29x times slower.

In a cold start (after node_modules installation and w/o vite cache), before Pigment, it was ~3s, but with Pigment, we currently need to start a vite server 2-3 times before it's working (related auto closed issue), so it's ~50s to start, then it becomes ~23s for a hot start.

[vite] Pre-transform error: ENOENT: no such file or directory, open './node_modules/.vite/deps/@mui_material.js'

Overall, I'm happy with Pigment CSS, if you want my opinion. It outputs CSS, resulting in a noticeable performance improvement comparing to emotion on even the latest iPhone, when there is intensive animation with elements being added/removed in DOM.

  • please continue with Pigment CSS; I still believe stylex is a better option because I don't care about the ease of migration. stylex offers protection on what styles are accepted by components, forces stricter CSS, leading to less visual bugs and more standardization, and results in even faster UX due to atomic css outputs (as described above by nmn)

@oliviertassinari
Copy link
Member

@o-alexandrov interesting, I think we need to keep a close eye on this build time performance. It seems that this will be one of the key challenges to overcome before Pigment CSS can reach mainstream adoption. For now, we are at about 1/1000 of the market depth: https://npm-stat.com/charts.html?package=%40pigment-css%2Freact&package=%40stylexjs%2Fstylex&from=2023-07-18&to=2024-07-18. Now, we will see, maybe it won't be that much of a blocker to grow x10 or x100.

I suspect we will need to enforce more constraint into the API. For example, maybe we shouldn't support the import of values from other files, instead for developers to define those values once where they configure Pigment CSS. It would still be more flexible than Tailwind CSS or CSS Modules, and maybe this can unlock x10 faster build.

@siriwatknp
Copy link
Member

@o-alexandrov @oliviertassinari I agree, at this stage of Pigment CSS, the perf is not that great and I feel like this is the next thing to improve before v1. cc @brijeshb42

@siriwatknp
Copy link
Member

@o-alexandrov I think stylex may be a better option for some use cases but I don't think it's a way to go for MUI (I know that this issue discussed about this but just want to double down my thought). The constraints are red flags that I won't use even in my side-projects.

I think stylex might work for a very very big project that requires strict rule of how CSS is authored so that the projects don't go side way and leave legacy code to the next joiners.

For Pigment CSS (in my opinion), the value is clear to me, we just want to solve the runtime bottle-neck of Material UI without changing the API surface.

@brijeshb42
Copy link
Contributor Author

brijeshb42 commented Jul 25, 2024

in a medium size project, the hot start (after vite created optimized deps) was ~0.3 seconds, now it's ~10s, so it's about ~33x times slower.

@o-alexandrov If it's not too much trouble, can you enable debug mode for Pigment CSS and share the output files. You can inspect the output files before sharing but it will mainly contain file paths and some timing information and won't have any sensitive information.
You can enable by adding this to the Pigment config -

const pigmentOptions = {
  theme: yourTheme,
  debug: {
    dir: 'tmp',
    print: true,
  },
};

Then either zip and share the tmp directory or share the individual files directly through Github gist.

@o-alexandrov
Copy link
Contributor

o-alexandrov commented Jul 26, 2024

@brijeshb42 thank you, I added debug props you mentioned.

For cold start, the debug had no effect, whereas the project didn't bundle (minimal repro with few files in total available in this issue).
For hot start, it produced a tmp directory that I attached to this comment.

  • please note, there's less work for Pigment, due to transformSx being set to false (it doesn't bundle when set to true, and @mui/* isn't transformed, see related issue; so when you fix the bugs, the performance will be worsened at least 1.3x, due to the need to also transform @mui/* and sx props

      pigment({
        theme: props.theme,
        transformSx: false,
        transformLibraries: [`@company-name`],
        displayName: props.isDev,
        babelOptions: {
          // related issue: https://github.com/mui/pigment-css/issues/11
          plugins: [`@babel/plugin-transform-export-namespace-from`],
        },
      }),
  • I replaced in entrypoints.jsonl file the path to the monorepo that contains only frontend-related files, with .. So . at the beginning of a filename means the root of a repo.

  • btw, I made an error in my last msg, the hot start actually takes ~22s, not ~10s, as I thought before (almost the same timings with debug props removed);

    • first, vite consoles VITE v5.3.4 ready in 752 ms and then it's all Pigment (based on startedAt in the attached actions.jsonl)

Link to download .zip file

@o-alexandrov
Copy link
Contributor

o-alexandrov commented Oct 7, 2024

Just curious, what is the plan for Pigment CSS?
There’s pretty much no development since June 24th (releases). Not counting minor docs, OS-specific and other little fixes. This is good, but not addressing the major issues that overall the Pigment CSS isn’t working with Material UI library.

Are you waiting for something specific in wyw-in-js (ex. their effort to explore oxc to address performance issues), realized relying on a 3rd party is more beneficial, or anything else?

@mnajdova
Copy link
Member

mnajdova commented Oct 11, 2024

@o-alexandrov we are going to be focusing next on adding more Material UI integration examples with Pigment CSS, e.g. webpack integration and fixing bugs related to this.

In general, we are going to be focusing on some APIs addition we want to have for v1, which we are going to share soon in an RFC. The next big thing would be improving the build performance - for this we still don't have a definite conclusion on how we are going to do, it could be trying to move some things to Rust, or maybe replacing wyw-in-js.

We are also extending the team, looking for one more engineer to work on it: https://x.com/olivtassinari/status/1822981108255895619

Long term, for a future version of Material UI, we may look into bundling the styles for Material UI in the package - meaning we would use Pigment CSS in the build step of Material UI, so we don't as developers to transpile the code coming from node_modules.

These are some high-level updates, we are going to be sharing them in the public roadmap for Pigment CSS by the end of the year.

Let me know if you have more questions :)

@o-alexandrov
Copy link
Contributor

o-alexandrov commented Oct 11, 2024

@mnajdova please refer to this comment by @Anber

  • apologies for the ping, just hopefully it'll help you to combine efforts

I totally agree performance should be handled last; i.e. after fixing bugs that prevent to use PigmentCSS w/ MaterialUI and missing features.

  • shipping pre-bundled MUI's styles would help to overcome current inability to use MaterialUI with PigmentCSS
    • but it will probably result in inability to override MUI's styles at build-time, resulting in the duplication in the output

Main bugs, imho: #43750, #43722, there's no open issue for HMR that breaks very often, but it's hard to create a reproduction for it; if you use PigmentCSS in any small project, you would stumble on it
Main missing feature: multiple themes prefer-color-scheme as fallback

@jeltehomminga
Copy link

It would be helpfull if there would be an example of using Emotion and Pigment together in NextJs. If we use Pigment in the main layout and pages layouts it would unlock already features we can use with Server Components.

So I assume we need to have the Pigment setup in the layouts and then ThemeProvider for emotion wrapping individual pages. Is this a recommended setup for this? Will it work good together, any pitfalls?

@o-alexandrov

This comment was marked as off-topic.

@mnajdova

This comment was marked as off-topic.

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/* package: system Specific to @mui/system performance RFC Request For Comments
Projects
None yet
Development

No branches or pull requests