[styles] POC around the stable v4 API

[oliviertassinari]

This contains all the changes (some a breaking) I want to explore for the stable styles API of v4.

• Match JSS: rename generateClassName -> generateId. For reference: https://cssinjs.org/jss-api?v=v10.0.0-alpha.14.
• Make global Css "safe": rename: dangerouslyUseGlobalCSS -> globalClassNames. The limitations haven't changed since we first introduced this option. It's pretty much like using the classes API. I believe we can make it safe.
• Make the JSS compression optional in production. One of the great strength of Bootstrap is the ability to inspect the source and to reproduce based on it. Disabling the class name minification by default can help in this direction. By disabling it on the documentation website, we make it easier to inspect the implementation.
• Propose a new server-side API: ServerStyleSheets. It's an attempt to simplify the API: Simplify NextJs Example? Should we get the page context from renderPage? #14677
• Change the global class name output, use the kebab case convention. (MuiButton-root -> mui-button-root).
• Reduce the number of component used for styled (use makeStyles instead of withStyles).
• Propose to make any selector with a specificity > 1 to use a global class name. For instance (no breaking change): (bad idea)

      marginLeft: -1,
      borderLeft: '1px solid transparent',
    },
-   '&$selected': {
+   '&.selected': {
      color: theme.palette.action.active,
      backgroundColor: fade(theme.palette.action.active, 0.12),
      '&:hover': {
@@ -32,7 +32,7 @@ export const styles = theme => ({
        marginLeft: 0,
      },
    },
-   '&$disabled': {
+   '&.disabled': {
      color: fade(theme.palette.action.disabled, 0.12),
    },
    '&:hover': {
@@ -42,7 +42,7 @@ export const styles = theme => ({
      '@media (hover: none)': {
        backgroundColor: 'transparent',
      },
-     '&$disabled': {
+     '&.disabled': {
        backgroundColor: 'transparent',
      },
    },
@@ -102,8 +102,8 @@ class ToggleButton extends React.Component {
        className={clsx(
          classes.root,
          {
-           [classes.disabled]: disabled,
-           [classes.selected]: selected,
+           [`disabled ${classes.disabled}`]: disabled,
+           [`selected ${classes.selected}`]: selected,
          },
          className,
        )}

• Introduce a new injectFirst property so the CSS injection change can be trivial.
• Make the hash generation logic non hash based by default: it's too slow client side. The gain is on the server, it helps few people.

[eps1lon]

Make the JSS compression optional in production.

Not so sure about that one. I'd rather have css source maps. I fear people will abuse this to get deterministic class names.

I guess this won't be merged as-is but rather split up into multiple PRs? Otherwise this is to much to review. Especially

Propose to make any selector with a specificity > 1 to use a global class name. For instance (no breaking change):

I currently don't understand how this wouldn't be a breaking change and I'm still missing a minimal example for both APIs. Would like to discuss this separately.

dangerouslyUseGlobalCSS -> globalClassNames

I don't see the benefit here? How do you make this safe? Simply renaming doesn't magically solve any issues with CSS that CSS-in-JS solutions tried to solve. I feel like we're moving back to global CSS without having a discussion about it.

[oliviertassinari]

I guess this won't be merged as-is but rather split up into multiple PRs? Otherwise, this is to much to review. Especially

@eps1lon This pull request will be split into multiple chunks. I'm interested in making the whole story fit together & to collect early feedback with some concrete code.

Not so sure about that one. I'd rather have css source maps. I fear people will abuse this to get deterministic class names.

CSS 3 supports substring-matching selectors. So yes, people could do:

div[class^="MuiButton-root-"]

What would be the problem with people abusing it to get deterministic class names? Given the API we already expose, IMHO, all our CSS is already public, at the exception of few components that we label private and that we can keep nondeterministic. I was thinking of making globalClassNames = true the default. All it would do is enable a more intuitive customization API.

Yes, CSS source map would help: cssinjs/jss#469.

I currently don't understand how this wouldn't be a breaking change and I'm still missing a minimal example for both APIs. Would like to discuss this separately.

It's not a breaking change as it won't break people code. All our existing capabilities will continue to work. We can later introduce a deprecation to reduce the number of ways people can implement the same behavior.

At the consumption, if we move into this direction, the diff will be:

--- a/docs/src/pages/demos/tabs/CustomizedTabs.js
+++ b/docs/src/pages/demos/tabs/CustomizedTabs.js
@@ -59,7 +59,7 @@ const useStyles = makeStyles(theme => ({
       color: '#40a9ff',
       opacity: 1,
     },
-    '&$tabSelected': {
+    '&.selected': {
       color: '#1890ff',
       fontWeight: theme.typography.fontWeightMedium,
     },
@@ -67,7 +67,6 @@ const useStyles = makeStyles(theme => ({
       color: '#40a9ff',
     },
   },
-  tabSelected: {},
   typography: {
     padding: theme.spacing(3),
   },
@@ -89,21 +88,21 @@ function CustomizedTabs() {
       <Tabs
         value={value}
         onChange={handleChange}
-        classes={{ root: classes.tabsRoot, indicator: classes.tabsIndicator }}
+        className={classes.tabsRoot}
       >
         <Tab
           disableRipple
-          classes={{ root: classes.tabRoot, selected: classes.tabSelected }}
+          className={classes.tabRoot}
           label="Tab 1"
         />
         <Tab
           disableRipple
-          classes={{ root: classes.tabRoot, selected: classes.tabSelected }}
+          className={classes.tabRoot}
           label="Tab 2"
         />
         <Tab
           disableRipple
-          classes={{ root: classes.tabRoot, selected: classes.tabSelected }}
+          className={classes.tabRoot}
           label="Tab 3"
         />
       </Tabs>

Being able to use the className API over the classes API is an important win for styled components users. Also, reducing the number of required object to provide to style a component is a win for plain CSS users. What do you have in mind as a minimal example for both API? I believe the diff illustrate how it changes between the current API and the proposed one. Now, I'm wondering if we need to change the core components. This behavior can be fully implemented at the generateId() level.
However, I'm not very happy with this solution. It might be a poor compromise between fully deterministic classnames and non-deterministic. Global class names sounds simpler.

I don't see the benefit here? How do you make this safe? Simply renaming doesn't magically solve any issues with CSS that CSS-in-JS solutions tried to solve. I feel like we're moving back to global CSS without having a discussion about it.

Exactly, it's the whole point, to "go back" to global CSS. This preliminary pull request tries to seed this discussion. Now, not everything will be global, the class names generated when doing css = f(props) can't be public, the components without the name attribute won't be public either.
Now, it's not really a regression, we primarily introduced JSS to:

1. Use CSS, because inline style was make our life impossible for media queries, pseudo-classes, pseudo-elements.
2. Remove any toolchain dependency, no Babel plugin, no Webpack loader.
3. To support dynamic themes, as well as nesting, sharing variables.
4. To only inject the CSS needed on the page, leverage CSS tree shaking.
5. To make overrides simpler!

It hasn't for solving the global CSS problem, sure we went with the default JSS behavior, we were scared of committing to a public CSS API. But here we are, after the introducing of the classes API, we can argue that our styles API is already public.

cc @siriwatknp would it help?

---

There is another correlated problem I'm not sure how to solve. class names are case insensitive, the convention is to use kebab-case, and yet, we are using PascalCase.

[eps1lon]

What would be the problem with people abusing it to get deterministic class names?

Because we now add an additional constrain on our default generateClassId. The proposed change from MuiButton to mui-button would not be breaking since we specifically document that class names are not deterministic in production. Now we make this option available.

It's fragmenting our API again. We have a styling solution. Why offer two?

Exactly, it's the whole point, to go back to global CSS.

That should be properly discussed in an RFC not just served as-is.

[oliviertassinari]

We have a styling solution. Why offer two?

1. When I talk to people, they don't understand how it works. Some don't understand how specificity works, some
2. It would solve Able to reference nested component in Theme #15002.

[oliviertassinari]

That looks wrong? Did you mean

@eps1lon Thanks, I have fixed the diff.

This looks very rushed.

Yes, It's a draft, it's meant to go to the trash and to iterate on it.

No prior discussion, no pros/cons with correct diffs, random twitter convos without actual examples. All of this in the midst of making @material-ui/styles stable and preparing v4.

We can take this example: https://codesandbox.io/s/zxmxjlx34.

• Problem n°1: People have to create a class name provider. It was solved in v4 with

 import React from "react";
 import { render } from "react-dom";
 import { create } from "jss";
-import JssProvider from "react-jss/lib/JssProvider";
-import { createGenerateClassName, jssPreset } from "@material-ui/core/styles";
+import { StylesProvider, jssPreset } from '@material-ui/styles';
 import StyledComponentsButton from "./StyledComponentsButton";

 const styleNode = document.createComment("insertion-point-jss");
 document.head.insertBefore(styleNode, document.head.firstChild);

-const generateClassName = createGenerateClassName();
 const jss = create({
   ...jssPreset(),
   insertionPoint: "insertion-point-jss"
 });

 const App = () => (
-  <JssProvider jss={jss} generateClassName={generateClassName}>
+  <StylesProvider jss={jss}>
     <StyledComponentsButton />
-  </JssProvider>
+  </StylesProvider>
 );

 render(<App />, document.querySelector("#root"));

• Problem n°2: People have to create a new JSS instance. I'm proposing the following API in this PR:

 import React from "react";
 import { render } from "react-dom";
-import { create } from "jss";
-import { StylesProvider, jssPreset } from '@material-ui/styles';
+import { StylesProvider } from '@material-ui/styles';
 import StyledComponentsButton from "./StyledComponentsButton";

-const styleNode = document.createComment("insertion-point-jss");
-document.head.insertBefore(styleNode, document.head.firstChild);
-
-const jss = create({
-  ...jssPreset(),
-  insertionPoint: "insertion-point-jss"
-});
-
 const App = () => (
-  <StylesProvider jss={jss}>
+  <StylesProvider injectFirst>
     <StyledComponentsButton />
   </StylesProvider>
 );

• Problem n°3: We use a selector with a specificity of 6, it's hard for people to wrap their head around how to override it. What's specificity, how do I increase specificity? I have made [TextField] Reduce the specificity #14953 so people can do:

   & .outline {
     border-color: red;
   }
-  &:hover:not(.focused):not(.e):not(.e) .outline {
+  &:hover .outline {
     border-color: yellow;
   }
   &.focused .outline {

• Problem n°4: People have to inject class names to the TextField component, ok, but where? This one is hard. With the global class names, people can do:

-const StyledTextField = styled(({ className, ...props }) => (
-  <TextField
-    {...props}
-    InputProps={{
-      classes: {
-        root: className,
-        notchedOutline: "outline",
-        focused: "focused"
-      }
-    }}
-  />
-))`
-  & .outline {
-    border-color: red;
-  }
-  &:hover .outline {
-    border-color: yellow;
-  }
-  &.focused .outline {
-    border-color: green;
+const StyledTextField = styled(TextField)`
+  .mui-outlined-input {
+    & .mui-outlined-input-notched-outline {
+      border-color: red;
+    }
+    &:hover .mui-outlined-input-notched-outline {
+      border-color: yellow;
+    }
+    &.focused .mui-outlined-input-notched-outline {
+      border-color: green;
+    }
   }
 `;

[eps1lon]

Problem n°4

That's a different story now. You were talking about global CSS which affects every component. Something CSS-in-JS wants to avoid since of all the reasons that I don't want to reiterate.

I'll ignore this for now until it's ready for review. I'm still a bit skeptical about certain implementations but we'll see when it's ready

[oliviertassinari]

Something CSS-in-JS wants to avoid since of all the reasons that I don't want to reiterate.

I have updated my previous comment with the history of why JSS:

we primarily introduced JSS to:

1. Use CSS, because inline style was make our life impossible for media queries, pseudo-classes, pseudo-elements.
2. Remove any toolchain dependency, no Babel plugin, no Webpack loader.
3. To support dynamic themes, as well as nesting, sharing variables.
4. To only inject the CSS needed on the page, leverage CSS tree shaking.
5. To make overrides simpler!

It hasn't for solving the global CSS problem, sure we went with the default JSS behavior, we were scared of committing to a public CSS API. But here we are, after the introducing of the classes API, we can argue that our styles API is already public.

---

I'll ignore this for now until it's ready for review. I'm still a bit skeptical about certain implementations but we'll see when it's ready

@eps1lon Ok, I'm gonna start splitting this pull requests in different chunks. I'm gonna drop the &$focus -> &.focus diff as I don't like it:

I'm not very happy with this solution. It looks like a poor compromise (hybrid) between deterministic and non-deterministic classnames.

[kof]

Allowing styling with plain class names makes class names a public api. It has bad and good parts in it.
Good because you can override with plain CSS.
Bad because:

1. You make them part of the public API, which will not allow you to change any class name without making it a breaking change.
2. You would encourage dirty overrides from the outside instead of configuration based theming. Dirty because they will use all possible specificity tricks and reach into every component from the outside at any point of time, making their own code unmaintainable.

[oliviertassinari]

That's a different story now. You were talking about global CSS which affects every component. Something CSS-in-JS wants to avoid since of all the reasons that I don't want to reiterate.

@eps1lon I don't think that people doing CSS-in-JS care about the global or not nature of their CSS explicitly, they like the side effect. I believe most of the value comes from:

1. You don't have to worry about CSS conflicts. You can move and delete styles with no problem.
2. You can write your styles very close to the component.

I don't think that Global CSS will change these two value propositions.

It's fragmenting our API again. We have a styling solution. Why offer two?

You are right, it's my biggest concern. I do think that both API have their own use cases:

• The classes is great when overriding with withStyles(), a bit less true with makeStyles(). It removes the need to tap into global class names.
• If you don't provide a name option, the classes object is the only way to override your components. I would still consider it the best and encouraged approach.
• The global CSS is great for styled-components, emotion or plain CSS users. Basically any solution outside of what we encourage.
• The global CSS API is the only way to reference component's styles between each other. It's not the first time I see this concern, it's frustrating users: Able to reference nested component in Theme #15002.

not allow you to change any class name without making it a breaking change.

@kof We already have this constraint, no?

You would encourage dirty overrides from the outside instead of configuration based theming.

overrides is an extension of configuration base theming when it's not enough. Configuration based theming should always be preferred.

[kof]

@kof We already have this constraint, no?

Currently it the property names in the theme. It is a bit more controlled since one can't nest things in unpredictable ways. But yeah, it's already there and it's not ideal since values are not strictly controlled for example shorthand properties.

overrides is an extension of configuration base theming when it's not enough. Configuration based theming should always be preferred.

I think once you open the chance of overriding using global classes, most will do that. Mb if it is at least not on by default and needs to be opted in and along with the flag we say this is not recommended.

[oliviertassinari]

react-select is an interesting study case. It's a project that moved from global CSS to emotion. (Material-UI did global LESS -> inline styles -> JSS).

• It started with people asking for a better class name structure: A more flexible approach to styling react-select ? JedWatson/react-select#1679 (42 👍)

What would be awesome would be to use a BEM approach to define a set of classes respectful to the DOM hierarchy of the component -> Which would be possibly overridden with a "theme" prop.

That would have the good benefit to let users extend styles of the component with css modules, or just using a more clear structure like BEM if he wants to stick with css.

I really like the way react-autowhatever deals with the issue, see : https://github.com/moroshko/react-autosuggest#theme-optional
Possibly a good source of inspiration.

The react-autowhatever's approach he is mentioning is what we are already doing with the classes object. The difference is that react-autowhatever provides global class name people can tape into, we don't. I really like this approach. It's what I'm leaning toward.
💥 Think of this as a classes API on steroid, an extension with nice default values, vs no default values.

JedWatson/react-select#1679 (comment)

This comment is interesting, how is styled-components making overrides better? Could it be the other way around? No explanation of why and how.

• After making the change and releasing v2, people started asking for disabling emotion: [V2] Option to disable CSS-in-JSS entirely JedWatson/react-select#2706 (79 👍).

I realize that it's probably not going away, and assuming that is a non-starter, can we have the ability to just disable emotion, and do all the styling ourselves? That will be painful, but at least there will only be one authority.

People prefer to start from a reasonable simple styled state than from scratch.

This is as well blocking me from using react-select. I have all styles compiled to one style.css file so I would love to have an option to get rid of inline styles and see some normal classes not css-1hwfws3.

People are targeting css-1hwfws3 in their CSS, that's funny 😆.

If we consider the number of upvotes in the issues (42 vs 79), moving to CSS-in-JS could have made things worse 🤔?

[oliviertassinari]

I think once you open the chance of overriding using global classes, most will do that. Mb if it is at least not on by default and needs to be opted in and along with the flag we say this is not recommended.

@kof There are 3 layers. Right now, it's a dangerous option, it has been like this for 2 years. It has always worked with no overhead. Should we make it safe? Should we make it the default? I'm leaning toward making it the default. It's the strategy used by Bootstrap, Ant Design, Vuetify, eleme.io, 4 massively used libraries. It's not all bad.

[kof]

If we consider the number of upvotes in the issues (42 vs 79), moving to CSS-in-JS might have made things worse

This is easy to conclude. In the end we know nothing about those users and their use cases. It is very different depending on skills level, time constraints, maintainability goals.

My assumption is that our industry in its majority is populated with short-term projects and beginner level devs. That's why the fastest to learn API will find more users, even if it is worse.

[kof]

Should we make it the default?

I didn't know we have that option. Do people fail to find it? Mb it's the placement in documentation?

[siriwatknp]

There is another correlated problem I'm not sure how to solve. class names are case insensitive, the convention is to use kebab-case, and yet, we are using PascalCase.

I think MuiButton-root is already well-defined. However, kebab-case cause me a lot of confusion sometimes in the developer console (when there is a lot of -).
Some project might already use Global CSS. I'm one of them, so changing to mui-button-root can broke the app.

[jasondashwang]

Problem n°3

I personally had a lot of difficulties with this one - so anyway of simplifying specificity on some commonly override classes would be an improvement.

Problem n°4

This one would be a major improvement in terms of being able to understand where the injection point happens (something both my team and I had difficulty understanding the semantics of); however, I think the larger issue that you briefly mentioned is that there should be a CLEAR encouraged approach - whether it is withStyles or makeStyles since they both cause different side effects.

For a lot of beginners - since I had to onboard a quite a few, their natural inclination was to try to use CSS in JS to override styles of a component rather than classes.

[siriwatknp]

I want to clarify this point. I think the biggest problem of JSS right now is that we cannot override nested styled children in parent?

This sandbox can explain the issue. If we create new custom component using withStyles api. I don't see how we will be able to customize it in the parent. Please look at Child.js and Parent.js then App.js
https://codesandbox.io/s/ywwx4lmp09

Does anyone have an idea to solve this problem ? without using global CSS would be great because I prefer writing sth like this

{
   MuiList: {
      root: {
         MuiListItem: ...
      }
   }
}

than

{
   MuiList: {
      root: {
         '& .MuiListItem': ...
      }
   }
}

cc @kof

[oliviertassinari]

I'm closing for #15140.

[ryancogswell]

I want to clarify this point. I think the biggest problem of JSS right now is that we cannot override nested styled children in parent?

This sandbox can explain the issue. If we create new custom component using withStyles api. I don't see how we will be able to customize it in the parent. Please look at Child.js and Parent.js then App.js
https://codesandbox.io/s/ywwx4lmp09

@siriwatknp Here's a modified version of your sandbox with one approach: https://codesandbox.io/s/5zo9mm3y6x

This assumes the child accepts a className prop and puts it on its root DOM element, but you could instead target the child DOM element type (e.g. "li") rather than using a class name.

Parent.js

import React from "react";
import { withStyles, List } from "@material-ui/core";
import Child from "./Child";

const styles = theme => ({
  list: {
    padding: 0,
    "&> $listItem": {
      marginTop: 16
    }
  },
  listItem: {}
});

const Parent = withStyles(styles)(({ classes }) => (
  // How to override child from parent ?
  <List className={classes.list}>
    <Child className={classes.listItem} />
    <Child className={classes.listItem} />
    <Child className={classes.listItem} />
  </List>
));

export default Parent;

Child.js

import React from "react";
import { withStyles, ListItem, ListItemText } from "@material-ui/core";
import classnames from "classnames";

const styles = theme => ({
  listItem: {
    backgroundColor: "#e5e5e5"
  },
  primary: {
    color: theme.palette.primary.main
  },
  secondary: {
    color: "#000000"
  }
});

const Child = withStyles(styles)(({ className, classes }) => (
  <ListItem className={classnames(className, classes.listItem)}>
    <ListItemText
      classes={{
        primary: classes.primary,
        secondary: classes.secondary
      }}
      primary={"Hello World"}
      secondary={"Subtitle"}
    />
  </ListItem>
));

export default Child;