mui-x
mui-x copied to clipboard
DataGrid should work with theme scoping (e.g. theme-ui)
Steps to reproduce
Link to live example: https://codesandbox.io/s/youthful-chaplygin-lw7d8y?file=/src/demo.tsx
Steps:
- Create a React app with
@mui/materialand@mui/x-data-grid. - Do the recommended setup for theme scoping from official MUI docs:
<ThemeProvider theme={{ [THEME_ID]: materialTheme }}>
{/* components from another library and Material UI */}
</ThemeProvider>
- Add the
<DataGrid>and click on e.g. a checkbox in the data grid.
Current behavior
If you click on e.g. a checkbox in the data grid, the app will break.
Expected behavior
It should work and use the provided theme, just as other components in @mui/material.
Context
With MUI it's possible to do theme scoping to avoid issues when using multiple styling solutions in one app, e.g. theme-ui. See: https://mui.com/material-ui/guides/theme-scoping/#using-with-theme-ui
Example:
import { ThemeUIProvider } from 'theme-ui';
import { createTheme as materialCreateTheme, THEME_ID } from '@mui/material/styles';
const themeUITheme = { ... };
const materialTheme = materialCreateTheme();
function App() {
return (
<ThemeUIProvider theme={themeUITheme}>
<MaterialThemeProvider theme={{ [THEME_ID]: materialTheme }}>
Theme UI components and Material UI components
</MaterialThemeProvider>
</ThemeUIProvider>
);
}
Hey @jln-dk, thank you for using MUI X DataGrid and reporting the issue related to theme scoping.
It's the second time in a while we've seen an issue related to the theme scoping, here's the previous one https://github.com/mui/mui-x/issues/10430 (already fixed)
Here's how to fix it:
diff --git a/packages/grid/x-data-grid/src/components/GridSelectedRowCount.tsx b/packages/grid/x-data-grid/src/components/GridSelectedRowCount.tsx
index c389b75a0..81f463f34 100644
--- a/packages/grid/x-data-grid/src/components/GridSelectedRowCount.tsx
+++ b/packages/grid/x-data-grid/src/components/GridSelectedRowCount.tsx
@@ -2,7 +2,7 @@ import * as React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { unstable_composeClasses as composeClasses } from '@mui/utils';
-import { styled, SxProps, Theme } from '@mui/system';
+import { styled, SxProps, Theme } from '@mui/material/styles';
import { useGridApiContext } from '../hooks/utils/useGridApiContext';
import { getDataGridUtilityClass } from '../constants/gridClasses';
import { useGridRootProps } from '../hooks/utils/useGridRootProps';
Feel free to open up a PR with a fix if you want to see this specific item fixed faster.
For the long term fix:
I guess we should update all instances of @mui/system to @mui/material/styles to avoid theme scoping issues in more components.
Unless we plan to fix it in @mui/system. 🤔
@siriwatknp please let us know if you have some knowledge about any such plans. CC @mui/xgrid
This would basically undo https://github.com/mui/mui-x/pull/8032
So for me we want to keep @mui/system and make it compatible with theme scoping
It appears that the issue lies with the use of a div element for GridSelectedRowCountRoot, as it is not properly integrated with theme scoping.
fix demo: https://codesandbox.io/s/nice-moore-wsp7q8?file=/src/demo.tsx
I wonder how does it work with the variant from @mui/material/styles then 🤔
I wonder how does it work with the variant from @mui/material/styles then 🤔
Most of the components use material-ui components, but the ones that are using div and other HTML elements crash.
If I am getting it right, I think that a proper solution to this problem will be one of the following:
- Switch back to the
@mui/material/styles(inverse of #8032) - Provide material theme and scoping support on the
@mui/systemvariant styled from the core side. - Have a wrapper around
@mui/systemstyled in the grid repo, include the material theme and use it instead of importing from either of the places (basically replicate part of what's internally being done in @mui/material variant of styled), but it will partially solve the problem (the theme part) as the scoping works based on theTHEME_IDexported from @mui/material, so unless we move this id to a common place (like @mui/system for example) and import from there everywhere, it is not likely to work as expected
Let me clarify how the theme scoping works at the moment and we can discuss the way to go for MUI X components.
@mui/system provides a factory function createStyled that can receive themeId to be able to scope the theme if needed.
@mui/material and @mui/joy use createStyled to produce styled API with a unique theme id. The styled is used to build components and also export from the package. All of the theming API will try to get theme.[themeId] if it exists.
The root cause of the error is the styled used by the DataGrid component comes from the system (no themeId), so when theme scoping is created, the DataGrid components still access the root theme which results in an error since all of the data is in theme.[themeId].*.
I think DataGrid should create it's own styled using createStyled from @mui/system with the same theme ID from Material UI (a hard-coded value without importing). This will fix the issue but it won't work with Joy UI in the future.
Here is what it looks like:
// styles/styled.ts
+ import { createStyled } from '@mui/system';
+ // hardcode theme id to avoid importing from @mui/material
+ const styled = createStyled({ themeId: '$$material' });
+ export default styled;
diff --git a/packages/grid/x-data-grid/src/components/GridSelectedRowCount.tsx b/packages/grid/x-data-grid/src/components/GridSelectedRowCount.tsx
index c389b75a0..a0218a542 100644
--- a/packages/grid/x-data-grid/src/components/GridSelectedRowCount.tsx
+++ b/packages/grid/x-data-grid/src/components/GridSelectedRowCount.tsx
@@ -2,7 +2,8 @@ import * as React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { unstable_composeClasses as composeClasses } from '@mui/utils';
-import { styled, SxProps, Theme } from '@mui/system';
+import { SxProps, Theme } from '@mui/system';
+import styled from '../styles/styled';
import { useGridApiContext } from '../hooks/utils/useGridApiContext';
import { getDataGridUtilityClass } from '../constants/gridClasses';
import { useGridRootProps } from '../hooks/utils/useGridRootProps';
@siriwatknp's This makes sense in the immediate 👍.
About the long term:
- What if the theme scoping was done at the ThemeProvider level? Not in the public API? It could behave as if there were different React contexts. Maybe we didn't do this because of the breaking change impact. People using emotion raw would no longer be able to tap into the Material UI theme.
- With @brijeshb42 work on runtime-less CSS-in-JS, I imagine the problem of nesting Joy UI with Material UI is going to be trickier to solve. Theme UI, Charka is pretty much solved, but How would a data grid container know if it needs to use Joy UI spacing of 10px or Material UI spacing of 8px? My best guess is that we would need to duplicate all the components. We would have a
<GridSelectedRowCountRoot>for Material UI, and a<GridSelectedRowCountRoot>for Joy UI. In this case, maybe we should already start to move in this direction.
Hi, is there a workaround for this as an MUI consumer while we wait for a fix to be released?
Hi @oliviertassinari 👋 Do you know if there has been any movement on this? The issue here is blocking us from integrating some components we've made using another library. I'd be happy to raise a PR to implement what @siriwatknp proposes but I'm not sure if that's the direction you want to go in. Could you advise please?
Hi @oliviertassinari 👋 Do you know if there has been any movement on this? The issue here is blocking us from integrating some components we've made using another library. I'd be happy to raise a PR to implement what @siriwatknp proposes but I'm not sure if that's the direction you want to go in. Could you advise please?
I added a comment here. You can follow the PR.
Are there any updates coming from this? I see that #10498 hasn't had any commits in 7 months and the last comment was 3 months ago. Has this stalled out?
In the meantime, are there any workarounds that consumers can apply?
I'm also seeing #12432 is open in a draft state and guessing that this relates to this comment. If this is the route, is there an ETA? This is a blocking issue for adopting DataGrid which we would like to do.
No updates, we plan to remove material-ui bundling but we don't have an ETA and I don't understand the implications for theme scoping which I haven't worked with yet. Maybe @cherniavskii or @MBilalShafi can provide more details on which workarounds could be used.