[styled-engine-sc] Fix types for css function of styled-engine-sc

[delijah]

• I have followed (at least) the PR section of the contributing guide.

Fixes #40498 and part 1 of #40140

Codesandbox: https://codesandbox.io/p/devbox/styled-engine-sc-css-function-types-49tz5g?file=%2Fsrc%2FApp.tsx%3A7%2C12

[mui-bot]

Netlify deploy preview

https://deploy-preview-40640--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against f2d8503

[mnajdova]

Can you please create a sandbox using the build packages from the PR, that will validate the fix? These are the package version:

• @mui/material: https://pkg.csb.dev/mui/material-ui/commit/ef9f9180/@mui/material
• @mui/styled-engine-sc: https://pkg.csb.dev/mui/material-ui/commit/ef9f9180/@mui/styled-engine-sc

You can find them here: https://ci.codesandbox.io/status/mui/material-ui/pr/40640/builds/461016

[delijah]

Can you please create a sandbox using the build packages from the PR, that will validate the fix? These are the package version:

To be honest: This codesandbox situation, when using styled-engine-sc, overwhelms me a bit. The parcel version does not work correctly and if i create a devbox myself, .. i could also not figure out how to get this working. Even created an issue some time ago: #40497.

Is there any existing, working, codesandbox out there, using typescript and styled-engine-sc, which i could re-use, in order to satisfy your request?

[delijah]

Can you please create a sandbox using the build packages from the PR, that will validate the fix? These are the package version:

Ok, somehow i was able to create something that might work 🙂

https://codesandbox.io/p/devbox/styled-engine-sc-css-function-types-49tz5g?file=%2Fsrc%2FApp.tsx%3A7%2C12

[delijah]

Let me know if i can be of any help in order to get this merged ? @mnajdova

[mnajdova]

Won't using DefaultTheme in this type mean that the theme in the css prop will have the wrong type? It won't be the Material UI's theme. Ideally I feel like we should export from here a "generator" for this type and re-exprot it from Material UI using the Material UI's Theme. We would do the same for the other package as well (@mui/styled-engine).

[delijah]

I basically agree. That is what my other PR is actually doing/solving.

The issue here is, the actual theme variable we receive within the css-function, is not the material theme. We have to use theme[THEME_ID] in order to access the MUI theme. So this problem already exists currently. But at least, with this fix/PR, we have solved the typing issue for variables. Step 2 would be getting the value and types of the theme prop right (which is solved in the other PR mentioned above).

[delijah]

As long as we do not fix the theme variable thing properly (other PR), i do not know what other type we could use here.

[brijeshb42]

I tried code in your repo and added a theme with a ThemeProvider. Added this extra code (disabled type-checking temporarily)

const complexMixin = css<ComponentProps>`
  color: ${(props) => {
    console.log(props);
    return props.$whiteColor ? "white" : "black";
  }};
`;

const StyledComp = styled("div")<ComponentProps>`
  /* This is an example of a nested interpolation */
  ${(props) => (props.$complex ? complexMixin : "color: blue")}
`;

and I found that the theme in props is actually logged correctly and does not need to be accessed using theme[THEME_ID]. Do you have a repro of the above case?

[siriwatknp]

@delijah I got your point. Would it make sense to do it in the same PR? This way we prevent the scenario where 2 PRs are not in the same release.

From what I see, both of your PRs do not contain huge changes so I think they can be combined.

[mnajdova]

Tagging @siriwatknp and @brijeshb42 to help with pushing this, together with https://github.com/mui/material-ui/pull/40656/files to the finish line. I won't have bandwidth to look into it this week, and I don't want to need to postpone it any further.