material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard

[system] Pass the stylesheet directly to `GlobalStyles`

Open siriwatknp opened this issue 1 year ago • 1 comments

To unblock https://github.com/mui/material-ui/pull/43708

Root cause

After the fix from #43632, if <StyledEngineProvider injectFirst> is used, the global style injection order is included in the Emotion normal process like styled API. Meaning that Emotion will process the end of the GlobalStyles first. For example,

https://github.com/user-attachments/assets/aaf48fd1-3568-480e-a489-2c11bf791880

This causes unexpected behavior because the way CSS variables are generated should be ordered by:

<GlobalStyles styles={{ :root }} />
<GlobalStyles styles={{ light }} /> // inject after dark, this is the bug in https://github.com/mui/material-ui/pull/43708#issuecomment-2346368396
<GlobalStyles styles={{ dark }} />

Fix

This PR put the ordered stylesheet into a single GlobalStyles so that the injection order does not matter.

https://github.com/user-attachments/assets/33d8b113-9e5e-409b-9698-9b277ab5b8a4


siriwatknp avatar Sep 13 '24 04:09 siriwatknp

Netlify deploy preview

https://deploy-preview-43739--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad) Details of bundle changes

Generated by :no_entry_sign: dangerJS against 054986f85a84bf259e2ada899a0a841c41fc7563

mui-bot avatar Sep 13 '24 06:09 mui-bot

@siriwatknp could we add a test for this?

Added a regression test (I think for use case like this, it's best to run the test in the real browser env). Also, tested with sandbox build, it fixes the issue.

https://app.argos-ci.com/mui/material-ui/builds/32120/109050219

siriwatknp avatar Sep 16 '24 03:09 siriwatknp