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

Various issues with MUI v6 Beta + Pigment + Remix #43380

Closed
sdriffill-tm opened this issue Aug 20, 2024 · 12 comments
Closed

Various issues with MUI v6 Beta + Pigment + Remix #43380

sdriffill-tm opened this issue Aug 20, 2024 · 12 comments
Assignees
Labels
package: material-ui Specific to @mui/material package: pigment-css Specific to @pigment-css/* v6.x

Comments

@sdriffill-tm
Copy link

sdriffill-tm commented Aug 20, 2024

Steps to reproduce

Link to live example:
forked repo branch

Current behavior

  1. my theme options aren't being applied i.e. the primary/secondary/error colors are not being overridden

  2. I'm getting browser console error Warning: Prop dangerouslySetInnerHTML did not match when running in dev

  3. When running the prod build, the sx prop styles on my components aren't being applied due to style tags in the head taking precedence - these style tags have the attribute data-emotion=... I've not sure why there is anything to do with emotion going on since I've followed the steps to replace with pigment.

  4. If I add the Stack component to my app I get error Internal server error: @pigment-css/react: You were trying to call "generateAtomics" function without configuring your bundler. Make sure to install the bundler specific plugin and use it. @pigment-css/vite-plugin for Vite integration or @pigment-css/nextjs-plugin for Next.js integration.. See the _index.tsx route for where I have commented out the use of this in my repo.

  5. Throwing a 500 error on the server causes several browser console errors and causes the app to render nothing. The first console error is: chunk-Q5V2KV5S.js?v=b9511ac1:15014 Uncaught DOMException: Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted is not a child of this node. at StyleSheet2._insertTag (http://localhost:5174/node_modules/.vite/deps/chunk-MHCFHIPD.js?v=b9511ac1:40:23)
    Visiting the "error" route demonstrates this problem

  6. (minor) Getting linting error "Unknown property 'sx' found eslint(react/no-unknown-property)" - maybe worth adding something to the migration guide about adding an ignore clause to the relevant rule, similar to the existing section for adding it the typescript types.

Expected behavior

  1. my theme options override the default

  2. no error

  3. emotion style tags not present

  4. Stack component works and does not trigger an error

  5. The error is handled by the ErrorBoundary defined in my root route, which displays the error with my customized styling from the theme options.

  6. (optional) addition to the docs

Context

I'm trying to upgrade a minimal template of Remix + Vite + MUI from v5 to v6 w/ pigment. I've followed the latest migration guides as well as implemented suggestions applied from this discussion where other users have had issues getting a working Remix template. Here is a summary of the changes made and the rationale:

  1. Uninstalled emotion dependencies, installed pigment dependencies and upgraded MUI dependencies to the latest Beta version (as per migration guide)

  2. Added a types file for TypeScript support (as per migration guide)

  3. Added import "@mui/material-pigment-css/styles.css"; to the top level of my app (the root route in Remix) (as per migration guide)

  4. Removed the ThemeProvider component from the top level of my app (as suggested here )

  5. Added pigment+config to my plugins in my vite config file. The pigment config includes:

    • Passing my theme options to the extendTheme method
    • Passing transformLibraries: ["@mui/material"]

    (as suggested here and now in the migration guide)

  6. Bundling MUI and pigment dependencies using the ssr.noExternal vite config option, due to CJS/ESM issues (as suggested in this example repo )

Your environment

npx @mui/envinfo

I used Chrome Version 127.0.6533.120 (Official Build) (64-bit)

System:
  OS: Windows 10 10.0.19045
Binaries:
  Node: 20.11.1 - ~\.nvm\versions\node\v20.11.1\bin\node.EXE      
  npm: 10.2.4 - ~\.nvm\versions\node\v20.11.1\bin\npm.CMD
  pnpm: Not Found
Browsers:
  Chrome: Not Found
  Edge: Chromium (127.0.2651.74)
npmPackages:
  @emotion/react:  11.13.0
  @emotion/styled:  11.13.0
  @mui/core-downloads-tracker:  6.0.0-dev.240424162023-9968b4889d 
  @mui/icons-material: ^6.0.0-beta.6 => 6.0.0-beta.6
  @mui/material: ^6.0.0-beta.6 => 6.0.0-beta.6
  @mui/material-pigment-css: ^6.0.0-beta.6 => 6.0.0-beta.6
  @mui/private-theming:  6.0.0-beta.6
  @mui/styled-engine:  6.0.0-beta.6
  @mui/system:  6.0.0-beta.6
  @mui/types:  7.2.15
  @mui/utils:  6.0.0-beta.6
  @types/react: ^18.2.20 => 18.2.60
  react: 18.3.0-canary-01ab35a9a-20240228 => 18.3.0-canary-01ab35a9a-20240228
  react-dom: 18.3.0-canary-01ab35a9a-20240228 => 18.3.0-canary-01ab35a9a-20240228
  typescript: ^5.1.6 => 5.3.3

Search keywords: remix v6 pigment vite

@sdriffill-tm sdriffill-tm added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 20, 2024
@sdriffill-tm
Copy link
Author

Just to postscript the above with a big thank you to the maintainers for this great project which I love using and can't wait for the benefits this upgrade will bring 👍

@siriwatknp
Copy link
Member

Just to postscript the above with a big thank you to the maintainers for this great project which I love using and can't wait for the benefits this upgrade will bring 👍

Thank you for reporting all the issues!

@siriwatknp siriwatknp added package: pigment-css Specific to @pigment-css/* and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 21, 2024
@pushys
Copy link
Contributor

pushys commented Aug 21, 2024

The fourth problem is probably connected to this issue I created in the Pigment CSS repo.

@5hal1n
Copy link

5hal1n commented Aug 26, 2024

I have same errors

@sdriffill-tm
Copy link
Author

Getting same issues with @mui/[email protected] and @pigment-css/[email protected]

@mc-dsk
Copy link

mc-dsk commented Sep 3, 2024

With a similar config, on latest version ([email protected] and "@pigment-css/vite-plugin": "0.0.21"), I get :

`[vite] Pre-transform error: @mui/material/FilledInput/FilledInput.js: Cannot convert undefined or null to object

vite] Pre-transform error: @mui/material/OutlinedInput/OutlinedInput.js: Cannot convert undefined or null to object`

@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
@brijeshb42
Copy link
Contributor

brijeshb42 commented Sep 9, 2024

Few updates while I try to fix this in dev. Let me know if you faces similar issues in production build as well -

  1. The final build did contain the updated values of your theme
Screenshot 2024-09-09 at 10 55 55 AM
  1. This is an issue in dev and I am working on it
  2. The emotion style tags were probably coming because you were using Container from @mui/material instead of @mui/material-pigment-css/Container. Layout components from @mui/material won't work with static extraction. So each layout component is available to be imported from @mui/material-pigment-css/<component-name>. Once I updated this import, all emotion related css vanished from the output.
  3. Could you try this again with latest versions of the libs. I was able to use both Stack and Container. If it is dev issue, please wait for an update from us.
  4. This is probably related to emotion. Will update.
  5. @siriwatknp Do we have this point covered in our docs ?

@brijeshb42
Copy link
Contributor

Finally found the issue to be Container import. If you just change the code to import it from @mui/material-pigment-css/Container, all the issues go away on dev and prod builds.
Let me know if this fixed the issue for you or if there are any other issues.

@brijeshb42 brijeshb42 added the status: waiting for author Issue with insufficient information label Sep 9, 2024
@sdriffill-tm
Copy link
Author

Thanks very much @brijeshb42, I'll take a look this morning and get back to you 👍

@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 Sep 9, 2024
@sdriffill-tm
Copy link
Author

  1. My theme options are now being applied in both dev and prod build 🎉
  2. This issue goes away when 3. is corrected 🎉
  3. My bad - missed this, was clearly listed in migration doc. Changed import and no ref to emotion in the document anymore. Also fixes 2. 🎉
  4. Both Stack and Container imports from pigment working. 🎉
  5. 500 server error now being handled as expected 🎉
  6. "react/no-unknown-property": ["error", { ignore: "sx" }] did the trick for me 🎉

From my perspective all issues resolved (other than 6. perhaps pending a doc update). I've pushed fixes to my example repo. Many thanks for your efforts @brijeshb42 and co 👍

@brijeshb42
Copy link
Contributor

Closing this then. Feel free to re-open if facing related issue.

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

github-actions bot commented Sep 9, 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 @sdriffill-tm! 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!

@github-actions github-actions bot removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: material-ui Specific to @mui/material package: pigment-css Specific to @pigment-css/* v6.x
Projects
Status: Done
Development

No branches or pull requests

8 participants