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 fix variant #41359

Closed
wants to merge 1 commit into from

Conversation

maapteh
Copy link

@maapteh maapteh commented Mar 4, 2024

closes #41356 not sure about other side effects since this was our only case where it didn't build anymore. Also im stumbled why typing errors are not shown in MUI's pipeline...

The fix still makes sure the intention of the previous PR (which introduced the bug) stays working correctly (choosing props that are only optional with the chosen variant).

So when setting a prop not belonging to the variant it shows the error:

Screenshot 2024-03-04 at 21 49 16

And when the prop is an option it allows it correctly:

Screenshot 2024-03-04 at 21 49 28

fixes #39137 (comment)

blamed #39137

What i still can do is to also change export type SelectProps back to export interface SelectProps because in general also this can be breaking ...

Signed-off-by: Mаартен - Maarten <maarten@oudenniel.nl>
@maapteh maapteh changed the title Update Select.d.ts [material-ui][Select] SelectProps fix variant Mar 4, 2024
@mui-bot
Copy link

mui-bot commented Mar 4, 2024

Netlify deploy preview

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

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 067a28c

@danilo-leal danilo-leal added component: select This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material labels Mar 5, 2024
@zannager zannager requested a review from mj12albert March 5, 2024 15:21
@DiegoAndai
Copy link
Member

Hey @maapteh, thanks for working on this!

The problem I see is that for the current implementation, the variant is not optional but required. If not provided, it would default to 'outlined' on runtime, so the type should be required for the StandardSelectProps and FilledSelectProps ensuring that the correct variant is used.

@DiegoAndai
Copy link
Member

We could make it optional for OutlinedSelectProps, as that's the default 🤔 would that fix your issue @maapteh?

I will have to investigate if that's a proper fix.

@ZeeshanTamboli
Copy link
Member

Will be fixed by #41405.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[material-ui][Select] SelectProps suddenly break by making variant required
5 participants