mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.54k stars 32.19k forks source link

Problem with the latest MUIv5 and classes overrides of nested elements #31035

Open synaptiko opened 2 years ago

synaptiko commented 2 years ago

Duplicates

Latest version

Current behavior 😯

Some of the MUI components have sub-components which are mentioned in the CSS section of the component with a statement like: "Styles applied to the deleteIcon element.". But when migrating from MUIv4 to v5, I found out that I cannot keep using it the same way in classes props because of increased specificity. There is no additional comment for such cases and it doesn't seem to be specific only to Chip component.

In the minimal example below I'm using makeStyles from tss-react, but I think the issue will be there even with other solutions.

const useStyles = makeStyles()((theme) => ({
  root: {
    backgroundColor: theme.palette.primary.main,
  },
  deleteIcon: {
    color: theme.palette.primary.contrastText,
  },
}));

function App() {
  const { classes } = useStyles();

  return (
    <Chip label="Chip with classes" onDelete={() => {}} classes={classes} />
  );
}

Expected behavior 🤔

Here's a demo page with actual & expected sections: https://muiv5-problem-with-classes.vercel.app/

In a nutshell, I would expect that I can pass deleteIcon in classes and be able to override color to whatever I need.

Steps to reproduce 🕹

https://github.com/synaptiko/muiv5-problem-with-classes/blob/master/src/App.tsx

FYI, I've created similar issue on tss-react as I initially thought it's related to that library: https://github.com/garronej/tss-react/issues/62.

Context 🔦

I'd like an easy way to override styles of nested elements/components, without guessing where specificity need to be increased. According to documentation my understanding was that classes is exposed for that purpose.

If it's not technically possible then I would at least expect some note in the CSS section of each component.

Sorry if it's a duplicate of https://github.com/mui/material-ui/issues/28664 but it seems slightly different to me. I can also provide other components for which I had the same issue.

Your environment 🌎

`npx @mui/envinfo` ``` System: OS: Linux 5.16 Arch Linux Binaries: Node: 17.3.0 - /usr/bin/node Yarn: 1.22.17 - /usr/bin/yarn npm: 8.4.1 - /usr/bin/npm Browsers: Chromium: 98.0 Firefox: 97.0 npmPackages: @emotion/react: ^11.7.1 => 11.7.1 @emotion/styled: ^11.6.0 => 11.6.0 @mui/base: 5.0.0-alpha.68 @mui/material: ^5.4.1 => 5.4.1 @mui/private-theming: 5.4.1 @mui/styled-engine: 5.4.1 @mui/system: 5.4.1 @mui/types: 7.1.1 @mui/utils: 5.4.1 @types/react: ^17.0.20 => 17.0.39 react: ^17.0.2 => 17.0.2 react-dom: ^17.0.2 => 17.0.2 typescript: ^4.4.2 => 4.5.5 ```
synaptiko commented 2 years ago

I found a workaround with use of tss-react on how to increase the specificity globally.

Note: It's hacky and not ideal, I'm not sure if it can eventually break some other things but I'll try to use it for now (to avoid unnecessary refactoring).

import { useTheme } from '@mui/material';
import { createMakeAndWithStyles } from 'tss-react';

const { makeStyles: _origMakeStyles, withStyles: _origWithStyles, useStyles } = createMakeAndWithStyles({ useTheme });

function addSpecificity(value: any): any {
  return { '&&': value };
}

function isSubcomponentName(key: string): boolean {
  return key !== 'root' && /^[a-z]+$/gi.test(key);
}

function increaseSpecificityOfSubcomponents(cssObject: Record<string, any>): any {
  const result: Record<string, any> = {};

  for (const key of Object.keys(cssObject)) {
    if (isSubcomponentName(key)) {
      result[key] = addSpecificity(cssObject[key]);
    } else {
      result[key] = cssObject[key];
    }
  }

  return result;
}

const withStyles: typeof _origWithStyles = (component, cssObjectOrGetterFn, params) => {
  if (typeof cssObjectOrGetterFn === 'function') {
    const origGetterFn = cssObjectOrGetterFn;

    cssObjectOrGetterFn = (...args) => {
      return increaseSpecificityOfSubcomponents(origGetterFn(...args));
    };
  } else {
    cssObjectOrGetterFn = increaseSpecificityOfSubcomponents(cssObjectOrGetterFn);
  }

  return _origWithStyles(component, cssObjectOrGetterFn, params);
};

function makeStyles<Params = void, RuleNameSubsetReferencableInNestedSelectors extends string = never>(params?: {
  name?: string | Record<string, unknown>;
}) {
  const origUseStyles = _origMakeStyles<Params, RuleNameSubsetReferencableInNestedSelectors>(params);
  const useStyles: typeof origUseStyles = (cssObjectByRuleNameOrGetCssObjectByRuleName) => {
    if (typeof cssObjectByRuleNameOrGetCssObjectByRuleName === 'function') {
      return origUseStyles((...args) =>
        increaseSpecificityOfSubcomponents(cssObjectByRuleNameOrGetCssObjectByRuleName(...args))
      ) as any;
    } else {
      return origUseStyles(increaseSpecificityOfSubcomponents(cssObjectByRuleNameOrGetCssObjectByRuleName));
    }
  };

  return useStyles;
}

export { makeStyles, withStyles, useStyles, _origWithStyles, _origMakeStyles }; // _origWithStyles/_origMakeStyles shouldn't be used, we just need to export it as well because of the typeof of withStyles/makeStyles

The deployed version can be seen here: https://muiv5-problem-with-classes-felxofanc-synaptiko1.vercel.app/

wilkinson4 commented 2 years ago

I am also experiencing this issue. Any updates?

synaptiko commented 2 years ago

It would be great to have an official fix or at least updated documentation with recommendations on to how to work with sub-components styling.

But for now we are using the workaround and so far it works fine.

wilkinson4 commented 2 years ago

It would be great to have an official fix or at least updated documentation with recommendations on to how to work with sub-components styling.

But for now we are using the workaround and so far it works fine.

Agreed. For the one place in our app it happened I just used inline styles to override the icon's color in the Chip as a temporary solution.

TheMoonDawg commented 1 year ago

Gross solution, but I just used the good ol' !important property to get the new color to take.

Hacky, but not awful. :)