Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[material-ui][Select] SelectProps suddenly break by making variant required #41356

Closed
maapteh opened this issue Mar 4, 2024 · 10 comments · Fixed by #41405
Closed

[material-ui][Select] SelectProps suddenly break by making variant required #41356

maapteh opened this issue Mar 4, 2024 · 10 comments · Fixed by #41405
Assignees
Labels
component: select This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material ready to take Help wanted. Guidance available. There is a high chance the change will be accepted regression A bug, but worse

Comments

@maapteh
Copy link

maapteh commented Mar 4, 2024

Steps to reproduce

In the latest version this PR https://github.com/mui/material-ui/pull/39137/files breaks the contract by suddenly making the variant prop a required one instead of the previous optional. This is not handy when extending on MUI components where the variant in the base props was optional.

Current behavior

Suddenly getting type errors when doing a patch update of MUI, it should not break

Property 'variant' is missing in type '...snip...' but required in type 'OutlinedSelectProps'.

Expected behavior

Not give compilation errors on components extending MUI Select

Context

When updating from 5.15.7 to 5.15.11 suddenly this error
I can omit the field, but i rather would like MUI to stick to its Base Input Props where variant is typed as optional.

Your environment

[5.15.7 -> 5.15.11] (https://renovatebot.com/diffs/npm/@mui%2fmaterial/5.15.7/5.15.11)

Search keywords: SelectProps variant

@maapteh maapteh added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Mar 4, 2024
@DiegoAndai DiegoAndai self-assigned this Mar 4, 2024
@DiegoAndai DiegoAndai added component: select This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material regression A bug, but worse and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Mar 4, 2024
@DiegoAndai DiegoAndai changed the title SelectProps suddenly break by making variant required [material-ui][Select] SelectProps suddenly break by making variant required Mar 4, 2024
@DiegoAndai
Copy link
Member

Hey @maapteh, thanks for the report.

Also reported here: #39137 (comment)

I'll look into it this week

@maapteh
Copy link
Author

maapteh commented Mar 4, 2024

Thank you, i looked but search didnt find anything. I will see if making them optional again will fix my issue.

FYI, this is breaking type change for people using SelectProps 
directly because the variant prop is required now when it wasn't 
before. I guess BaseSelectProps is a suitable replacement 
for most use cases?

This was helpful but not fixing the root cause:)

@maapteh
Copy link
Author

maapteh commented Mar 4, 2024

I created a fix in the PR attached to this issue report!

@maapteh
Copy link
Author

maapteh commented Mar 4, 2024

Please look at it @DiegoAndai when you find the time :)

@DiegoAndai
Copy link
Member

Hey @maapteh, may I ask you to provide a minimal reproduction? This would help a lot in debugging the issue. A live example would be perfect. This StackBlitz sandbox template may be a good starting point.

@maapteh
Copy link
Author

maapteh commented Mar 5, 2024

I solved it in #41359 right?

@DiegoAndai
Copy link
Member

I'm not sure, I left a comment there

@maapteh
Copy link
Author

maapteh commented Mar 5, 2024

Ah now i see, but its a union type so i dont see how that would work...
I created the sample at https://stackblitz.com/edit/stackblitz-starters-gfn7kp?file=src%2FApp.tsx

Screenshot 2024-03-05 at 23 48 10

@DiegoAndai
Copy link
Member

DiegoAndai commented Mar 5, 2024

I created a typescript playground with all the problems I could find, including this one:

@DiegoAndai
Copy link
Member

DiegoAndai commented Mar 7, 2024

I'll add the ready to take label to see if we get any contributors with ideas. This is the isolated repro: typescript playground. If you have an idea on how to support wrapping the Select component and avoid the errors in the playground, let's discuss here.

@DiegoAndai DiegoAndai added the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Mar 7, 2024
sai6855 added a commit to sai6855/material-ui that referenced this issue Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: select This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material ready to take Help wanted. Guidance available. There is a high chance the change will be accepted regression A bug, but worse
Projects
None yet
2 participants