[core] @mui/system has no singleton, it should a dependency

[oliviertassinari]

Steps to reproduce 🕹

Link to live example:

Steps:

1. Install charts https://mui.com/x/react-charts/
2. See the warnings

Current behavior 😯

mui-x/packages/grid/x-data-grid/package.json

Lines 55 to 57 in 9addfb0

	"peerDependencies": {
	"@mui/material": "^5.4.1",
	"@mui/system": "^5.4.1",

Expected behavior 🤔

https://github.com/mui/material-ui/blob/f1401844507519e09814bf8ea3ac338ec8011044/packages/mui-lab/package.json#L43-L46

emotion and styled-components are peer dependencies because they have a React theme context singleton. A duplication would break the application. In the case of the system, it's OK for the package to be duplicated in the bundle. It's not great for performance, but it would continue to work.

Context 🔦

Dependency first added in #3281. Bug discovered by @joserodolfofreitas

Your environment 🌎

npx @mui/envinfo

  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

[oliviertassinari]

A note about the future:

We are working toward making @mui/system a dependency developers have to install on their project, no longer available from @mui/material or @mui/joy. This npm package would also come with a theme singleton mui/material-ui#30660.
So we might need to revert the change I propose in this issue, but maybe not, it depends on how we approach the problem.

[oliviertassinari]

There is a singleton in @mui/system now (I just discovered it):

https://github.com/mui/material-ui/blob/45c0eca945d2ecf6add95934d8d181bc2e5b28c5/packages/mui-system/src/cssVars/createCssVarsProvider.js#L49

If we ever become to depend on it in MUI X, we would need to revert.

[cherniavskii]

Should we consider updating installation instructions instead?

-npm install @mui/x-charts
+npm install @mui/x-charts @mui/system

This way it will be deduplicated when other MUI packages are used.

[LukasTy]

There is a singleton in @mui/system now (I just discovered it):

https://github.com/mui/material-ui/blob/45c0eca945d2ecf6add95934d8d181bc2e5b28c5/packages/mui-system/src/cssVars/createCssVarsProvider.js#L49

If we ever become to depend on it in MUI X, we would need to revert.

@oliviertassinari Isn't the plan to go the route of only CSS variables when we move towards supporting RSC?
Not to mention, it could already be a problem if someone used CssVarsProvider currently in their app and has mismatched @mui/system dependencies. 🙈
https://github.com/mui/material-ui/blob/45c0eca945d2ecf6add95934d8d181bc2e5b28c5/packages/mui-material/src/styles/CssVarsProvider.tsx#L3

Should we just keep the existing behavior for now? 🤔

[oliviertassinari]

to go the route of only CSS variables when we move towards supporting RSC?

We can add @siriwatknp to the discussion as this is part of the code he owns.

From my perspective, we are moving towards a future where React context works with RSC, I don't see why they won't implement it, it not being supported isn't part of the initial RSC RFC, it seems to only be a feature that was left on the side to save time. But theoretically speaking, it should be "easy" for a framework to implement, as long as the values in the context can be serialized or are designed to be separate (one context for server, one for client).

I think #10427 (comment) is the future that we are trending towards (say 18 months from now), where if you use MUI X Charts with Material UI, you need @mui/system and @mui/material (system no longer being exposed from @mui/material, to distinguish them and to simplify multi-theme support with @mui/joy), but if you use MUI X Charts with Shadcn UI, you need tailwindcss.

Personally, I think we can move slower, start by fixing what's wrong today, maybe the future will be different.

[flaviendelangle]

as long as the values in the context can be serialized

That one is not a small assumption though