[system] Remove dependency on overridesResolver for the variants

[mnajdova]

I noticed this issue when working on a test case for #27847

Here is a codesandbox that illustrates the issue - https://codesandbox.io/s/ancient-snow-45mhz?file=/src/App.js Note that the styles from the variants are being applied only if the overridesResolver is defined. These are not related and this dependency is not expected.

[mnajdova]

Fixed wrong description on the test.

[mui-pr-bot]

Details of bundle changes (experimental)

Generated by 🚫 dangerJS against 845fb6b

[eps1lon]

What do you mean by "skipped" here. The test looks like it's not using variant in any way and is also passing without the change.

[eps1lon]

Should you maybe use a theme that has styles for this specific variant?

[mnajdova]

Yes, it is used on line 322 https://github.com/mui-org/material-ui/pull/27859/files#diff-57eedb98ff493d66eb3e22d9e9ef1b1540504af969aa62accbb30b3a2a780044R322 The same theme is used in all tests. It is defined in https://github.com/mui-org/material-ui/blob/5585e100feec585f40f42ba2db605a56d1594718/packages/material-ui-system/src/styled.test.js#L140

Previously the variants from the theme were not applied, if when creating the component, the overridesResolver option was not applied. That's what I mean by should not be skipped.

[eps1lon]

Previously the variants from the theme were not applied, if when creating the component, the overridesResolver option was not applied.

But that's not related to the change in implementation in this PR. I ran the test with yarn test:karma and in JSDOM locally on next and it is passing.

This is confusing long-term because usually tests are coupled with the change in implementation. But here the change in implementation and test are unrelated.

Maybe you need to assert on different CSS properties to showcase what behavior changed?

[mnajdova]

I see your point. I've fixed the tests. I was adding tests for a non root slot, but then variants are never applied :)

[eps1lon]

Here is a codesandbox that illustrates the issue - codesandbox.io/s/ancient-snow-45mhz?file=/src/App.js

Did you mean

 const Button = styled("button", {
   name: "MuiButton",
   slot: "Root",
   // overridesResolver: (props, styles) => styles.root,
-  shouldForwardProp: (prop) => prop !== "color" && prop !== "contained"
+  shouldForwardProp: (prop) => prop !== "color" && prop !== "variant"
 })({
   display: "flex"
 });

?

[mnajdova]

Did you mean

 const Button = styled("button", {
   name: "MuiButton",
   slot: "Root",
   // overridesResolver: (props, styles) => styles.root,
-  shouldForwardProp: (prop) => prop !== "color" && prop !== "contained"
+  shouldForwardProp: (prop) => prop !== "color" && prop !== "variant"
 })({
   display: "flex"
 });

?

Good catch, yes I've updated the codesandbox. In the codesandbox if you uncomment line 23, you will see that the variants are applied. I think this is surprising that the two are related.

[eps1lon]

e30e28e: Just wanted to check what "restore file" does in the GitHub UI and it actually restores the file directly 😆

[mnajdova]

Just wanted to check what "restore file" does in the GitHub UI and it actually restores the file directly 😆

This is useful when you want to revert changes to some files on a PR from a phone :)) Where do you see this option?

[eps1lon]

Where do you see this option?

Might be from https://github.com/sindresorhus/refined-github

[mnajdova]

Might be from https://github.com/sindresorhus/refined-github

It could be I can't see that option. Thanks for sharing!