[styles] Dynamic makeStyles unexpected behavior

[jmdsg]

• This is not a v0.x issue.
• I have searched the issues of this repository and believe that this is not a duplicate.

I have found some issues using makeStyles with dynamic styles (arrow functions) and i'm going to list them here since i think those may be related, and i feel that is unnecessary to create one issue per each of these points.

This issues are shown in this codesandbox MCVE https://codesandbox.io/s/lrwvw5lx6q.

1. The class name of a dynamic style contains 2 classes, the first is the represents the static class (hence is empty) and the second the dynamic class (with the actual styles).
   This behavior may be the intended to allow the reference of a static class from a dynamic class but i don't think is working, at least not completely.
2. Cannot mix or reference (with $) a static class (not using arrow function) from a dynamic class (using arrow).
3. Using some complex classname (not sure exacly which ones cause this behavior) when the component is rerendered (by a state change for instance) the previous style is not removed and the style sheet gets polluted and starts to have duplicate classes.
4. Cannot use a dynamic keyframe (using arrow function).
5. Cannot use/reference a static keyframe (just as an object) from a dynamic class. This may be related to (2).
6. And the last is more a question, i checked the code for makeStyles and i see that it does not depend on props, everytime useStyles is called and no matter if the props are the exacly the same the stylesheet is updated. Any reason for this?

Because when you call useStyles(props) with the same props or even using something like this:

const {
    height,
    width,
    ...rest
} = props;

// using only props that affect the styles generated
const memoizedStyleProps = useMemo(() => ({ height, width }), [height, width]);
const classes = useStyles(memoizedStyleProps);

Doesn't seem necessary to update the stylesheets.

Expected Behavior

1. Just one class name per dynamic style, e.g. makeStyles-animate-10
2. Be able to reference a static class from a dynamic class
3. Be able to rerender the component with complex classes without ending up with duplicate css rules in the style sheet.
4. Be able to use a key frame as a result of an arrow function.
5. Be able to use a static key frame from a dynamic style.

Current Behavior

1. Two class names per dynamic style, e.g. makeStyles-animate-4 makeStyles-animate-10
2. Cannot reference a static class from a dynamic class. In the example shows index.js:26 Warning: [JSS] Could not find the referenced rule simple in makeStyles.
3. Using a complex classname and rerendering the component causes the style sheet gets polluted
4. Not able to use a key frame as a result of an arrow function.
5. Not able to use a static key frame from a dynamic style. In the example shows index.js:26 Warning: [JSS] Referenced keyframes rule "rotate1" is not defined.

Steps to Reproduce

1. Inspect any of the elements using a dynamic class name and it will have a two classnames for.
2. In the example, if you change the definition of the class simple from simple: {} to simple: () => ({}) the title Hello CodeSandbox will have the expected green color.
3. Click one of the buttons in the example multiple times and then either inspect the h2 containing You clicked X times! or checking the rules in the sandbox document.styleSheets there will be multiple rule definitions repeated.
4. In the example the keyframe named rotate2 that is inside an arrow function is generated with empty body. The animation is not being applied.
5. In the example the keyframe named rotate1 is generated correctly but it is not referenced. The animation is not being applied.

Your Environment

Tech	Version
Material-UI	v4.0.0-alpha.8 and v4.0.2
React	16.8.0
Browser	Chrome: version 73.0.3683.103 (Build official) (64 bits)
TypeScript	No

Not sure if all this points are really issues or some of them are not allowed and its behavior is undefined.

[oliviertassinari]

cc @kof

[jmdsg]

Any news on this?

[joacub]

Same here is called too many times unnecessary

[Fleuv]

Encountered this problem as well.

I noticed that this problem only occurs when using a nested selector. So adding a separate class to the element the nested selector points at and adding the dynamic styling to that class should solve the problem.

In this code sandbox I debugged the problem:
https://codesandbox.io/s/styling-duplicates-on-state-change-0weti

Note: In my browser (FireFox) I had to close and open the inspector to see the affect, thus make sure you follow the debug instructions in the demo.

Conclusion

This doesn't work:

myParentElement: (props) => ({
  '& .myNestedElement': {
    // use props
  }
})

This does work:

myNestedElement: (props) => ({
  // use props
})

[warren-liang-exa]

Is there any update on this issue? Still getting more than one class when using props for adaptive styling.

[schnerd]

Also interested in item 6 – makeStyles shouldn't re-calculate and re-attach styles unless the props change.

I'd imagine it can be fixed with a small change like schnerd@21f98bd.

[oliviertassinari]

These issues should have been solved in v5 thanks to #22342 and the new @material-ui/styled-engine package. You can play with it at: https://codesandbox.io/s/bold-goodall-2iltv?file=/src/App.tsx. By default, it wraps emotion.