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
752 stars 38 forks source link

Bugs with incremental adoption alongside styled-components #177

Open aleksakojadinovic opened 3 months ago

aleksakojadinovic commented 3 months ago

Steps to reproduce

Test by visiting the homepage of this minimal Next.js application https://github.com/aleksakojadinovic/pigment-sc-reproduction/

Current behavior

When using both pigment css and styled-components in a Next.js app with pages router, an error regarding the styled component you defined occurs in some scenarios. In my case I created a styled component from h3 and imported it into a component that uses pigment styling as well, and this is the error:

EvalError: _styledComponents.default.h3 is not a function

I would assume this has something to do with transpilation - pigment picks up components styled with styled-components as well and attempts to transpile their code, which inevitably fails.

Expected behavior

Ideally pigment should not interfere with components created with styled-components. This would enable incremental adoption.

Context

I apologize in advance if this is not supposed to be considered a bug. I have searched the docs and have not managed to find out whether incremental/gradual adoption of pigment alongside styled-components is something you wish to support at all. However that's the situation that I'm in - I have a huge Next.js app very strongly coupled with styled-components that I wish to migrate to pigment, and doing it all at once is impossible. I've managed to isolate the problematic code into a minimal example, but I suppose many more cases would fail.

Your environment

npx @mui/envinfo ``` System: OS: macOS 14.3.1 Binaries: Node: 20.10.0 - ~/.nvm/versions/node/v20.10.0/bin/node npm: 10.2.3 - ~/.nvm/versions/node/v20.10.0/bin/npm pnpm: Not Found Browsers: Chrome: 126.0.6478.127 Edge: Not Found Safari: 17.3.1 npmPackages: @emotion/react: 11.11.4 @emotion/styled: 11.11.5 @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.15 @mui/utils: 6.0.0-dev.20240529-082515-213b5e33ab react: ^18 => 18.3.1 react-dom: ^18 => 18.3.1 styled-components: ^6.1.11 => 6.1.11 typescript: 5.5.3 ```

Search keywords: styled-components incremental-adoption

brijeshb42 commented 3 months ago

Here's the config that worked for me in your repro -

import { withPigment } from "@pigment-css/nextjs-plugin";
import { createRequire } from "node:module";

const require = createRequire(import.meta.url);

/** @type {import('next').NextConfig} */
const nextConfig = {
  reactStrictMode: true,
};

/** @type {import('@pigment-css/nextjs-plugin').PigmentOptions} */
const pigmentConfig = {
  theme: {
    colors: {
      primary: "#9542f5",
    },
  },
  tagResolver(source, tag) {
    if (source === "styled-components" && tag === "default") {
      return require.resolve("@pigment-css/react/exports/styled");
    }
    return null;
  },
};

export default withPigment(nextConfig, pigmentConfig);

Note the tagResolver option.

Then I changed your pages/index.js to have this -

import { SomeStyledComponent } from "../styles";

const DestinationsSection = () => {
  return (
    <div>
      test
      <SomeStyledComponent>Hello</SomeStyledComponent>
    </div>
  );
};
aleksakojadinovic commented 3 months ago

Here's the config that worked for me in your repro - Then I changed your pages/index.js to have this -

import { SomeStyledComponent } from "../styles";

const DestinationsSection = () => {
  return (
    <div>
      test
      <SomeStyledComponent>Hello</SomeStyledComponent>
    </div>
  );
};

But you removed the usage of pigment styled component which is the key part? I need both pigment and sc to work in the same component. This would've worked without the tagResolver you mentioned. When I apply this fix without changing pages/index, I get different errors:

unhandledRejection: ReferenceError: /Users/aleksakojadinovic/projects/pigment-issues-repro/src/styles.js: require is not defined at tagResolver (file:///Users/aleksakojadinovic/projects/pigment-issues-repro/next.config.mjs:16:7) at tagResolver (/Users/aleksakojadinovic/projects/pigment-issues-repro/node_modules/@pigment-css/unplugin/build/index.js:249:46) at getProcessorForImport (/Users/aleksakojadinovic/projects/pigment-issues-repro/node_modules/@wyw-in-js/transform/lib/utils/getTagProcessor.js:87:22) at /Users/aleksakojadinovic/projects/pigment-issues-repro/node_modules/@wyw-in-js/transform/lib/utils/getTagProcessor.js:273:38 at Array.forEach () at getDefinedProcessors (/Users/aleksakojadinovic/projects/pigment-issues-repro/node_modules/@wyw-in-js/transform/lib/utils/getTagProcessor.js:272:13) at applyProcessors (/Users/aleksakojadinovic/projects/pigment-issues-repro/node_modules/@wyw-in-js/transform/lib/utils/getTagProcessor.js:332:29)

danLDev commented 2 months ago

@aleksakojadinovic did you have any luck finding a fix for this? Looks like it could be related to some compatibility issues we're seeing with antd https://github.com/mui/pigment-css/issues/179

Not sure if it helps you, but we found that we could move the imports around & define the pigment styles in a separate file we were okay in some circumstances. Unfortunately this wasn't good enough for us due to the way our components are organised

aleksakojadinovic commented 2 months ago

Hi, unfortunately no, no luck in solving it. I proceeded by picking a bigger chunk of my app to migrate. This led me on a styled-components witch-hunt where I would

  1. Start the dev server
  2. Visit the page that I'm migrating
  3. A component crashes - I migrate it to pigment
  4. Restart the dev server because it hangs indefinitely on every crash
  5. Repeat

If the component was reused by another feature I would clone it and keep both pigment and SC variants.

In the meantime I found more blocking issues such as https://github.com/mui/pigment-css/issues/171, therefore I'm not sure whether I'll be continuing with pigment at all for now - seems too unstable. I'll keep this issue open anyway as it might help someone else.

oliviertassinari commented 1 week ago

Related to #180