mui / material-ui

MaterialΒ UI: Ready-to-use foundational React components, free forever. It includes Material UI, which implements Google's Material Design.
https://mui.com/material-ui/
MIT License
92.75k stars 31.94k forks source link

Types and Interfaces are ignored in mui/system styled function #37214

Open JoelBonetR opened 1 year ago

JoelBonetR commented 1 year ago

Duplicates

Latest version

Steps to reproduce πŸ•Ή

Link to live example:

Steps:

  1. Download this project:
    curl https://codeload.github.com/mui/material-ui/tar.gz/master | tar -xz --strip=2 material-ui-master/examples/material-cra-styled-components-ts
    cd material-cra-styled-components-ts
  2. npm install
  3. Copy the following code:
    
    import { styled } from '@mui/system';

interface ParagraphProps { color: 'green' | 'blue'; }

export const Paragraph = styled('p')(({ color }) => ({ color: color || '#000', fontSize: '18px', fontWeight: 'bold', }));

Paragraph({ color: 'red' });


### Current behavior 😯

No errors shown

### Expected behavior πŸ€”

Last line throws 

Type '"red"' is not assignable to type '"green" | "blue"'.ts(2769)


### Context πŸ”¦

I am trying to provide an Interface/type around which properties and values a MUI Styled Component should admit to avoid issues during the development process and its maintenance, which is one of the key points within the CSS-in-JS concept.

### Your environment 🌎

<details>
  <summary><code>npx @mui/envinfo</code></summary>

Don't forget to mention which browser you used. Output from npx @mui/envinfo goes here.


</details>
JoelBonetR commented 1 year ago

For reference/more info: https://stackoverflow.com/questions/76207234/cant-provide-interfaces-or-types-with-mui-styled-components

mnajdova commented 1 year ago

It looks to me that it has to do some of the typescript config, as using directly styled-components works - https://codesandbox.io/s/serene-frost-03vzju?file=/src/App.tsx. @samuelsycamore we should probably prioritize creating vite examples, if the config of module resolution works better.

JoelBonetR commented 1 year ago

@mnajdova I just checked with mui/system:

import { styled } from '@mui/system';

interface ParagraphProps {
  color: 'green' | 'blue';
}

export const Paragraph = styled('p')<ParagraphProps>(({ color }) => ({
  color: color || '#000',
  fontSize: '18px',
  fontWeight: 'bold',
}));

export default function App() {
  return <Paragraph color="red">Paragraph</Paragraph>;
}

inside the code sandbox and it looks good to me, maybe the issue is in the example config and not in the actual implementation?

For reference the example I checked the other day is mentioned here, below "How to use".

Just to add more information I'm using a Macbook Pro (M1 Pro) witch MacOSx (latest) and Node 18.15.0 running on my system.

mnajdova commented 1 year ago

For reference the example I checked the other day is mentioned here just below "How to use".

The example you mentioned uses @mui/styled-engine-sc which is our adapter for using styled-components instead of emotion. As you mentioned in the codesandbox it works as expected when using the default config. Was your intention to use styled-components? Was there particular reason for it (I am just curious :))?

JoelBonetR commented 1 year ago

For reference the example I checked the other day is mentioned here just below "How to use".

The example you mentioned uses @mui/styled-engine-sc which is our adapter for using styled-components instead of emotion. As you mentioned in the codesandbox it works as expected when using the default config. Was your intention to use styled-components? Was there particular reason for it (I am just curious :))?

Yes I wanted to take a gander on MUI with Styled-Componentes. Reasons being:

  1. The team already has experience of building fully customized UIs with SC since 3 years ago (with and without SSR) and has been quite nice overall.
  2. The doc says we should be able to.
  3. The main purpose of using a UI kit (either be MUI or any other) in our situation is to accelerate the delivery of a given product, in this situation if every team member needs to learn a different tool (emotion) in the chain diminishes the outcome.

I've been analysing other possible dependencies on this initial stage so I may come back in some days and try and build everything from scratch (no CRA, no curl-ing any example) and see if I can avoid the drama and get things done πŸ˜„

mnajdova commented 1 year ago

The main purpose of using a UI kit (either be MUI or any other) in our situation is to accelerate the delivery of a given product, in this situation if every team member needs to learn a different tool (emotion) in the chain diminishes the outcome.

Makes sense, my main point was, the API is almost identical :)

I've been analysing other possible dependencies on this initial stage so I may come back in some days and try and build everything from scratch (no CRA, no curl-ing any example) and see if I can avoid the drama and get things done πŸ˜„

This may even be a better option, but again using emotion you will be able to avoid all of this, this was the main reason for my question above :)

JoelBonetR commented 1 year ago

The main purpose of using a UI kit (either be MUI or any other) in our situation is to accelerate the delivery of a given product, in this situation if every team member needs to learn a different tool (emotion) in the chain diminishes the outcome.

Makes sense, my main point was, the API is almost identical :)

I've been analysing other possible dependencies on this initial stage so I may come back in some days and try and build everything from scratch (no CRA, no curl-ing any example) and see if I can avoid the drama and get things done πŸ˜„

This may even be a better option, but again using emotion you will be able to avoid all of this, this was the main reason for my question above :)

I know that is almost identical but not everyone bothers checking plus approve new stuff on big organisations is not so straightforward sometimes... πŸ˜…

Have you been able to validate whether is it something which origin is a specific config and/or that is unique to the example provided but not on usual project setup?

I may check it on my own and come back with as detailed as possible answers (or maybe a PR if there's something to fix), the forecast for the next few weeks is "busy as hell" so I can't promise I'll commit to it, tho πŸ˜“