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.

Sign up for GitHub

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

Jump to bottom

[TextField] Fix missing size prop in TypeScript types #18752

Merged
merged 1 commit into from
Dec 9, 2019
Merged

[TextField] Fix missing size prop in TypeScript types #18752

merged 1 commit into from
Dec 9, 2019

Conversation

sarpt
Copy link
Contributor

@sarpt sarpt commented Dec 8, 2019

Fixes #18743

sarpt
sarpt commented Dec 8, 2019
View reviewed changes
packages/material-ui/src/TextField/TextField.d.ts
@@ -36,6 +36,7 @@ export interface BaseTextFieldProps
required?: boolean;
rows?: string | number;
rowsMax?: string | number;
size?: 'small' | 'medium';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: would it be a good idea to make enums or types for string values as 'small' or 'medium' and then make a size combined type of Size.Small | Size.medium? I see many places use those inline values, which is a very minor thing but it leaves an opening for small typos

Copy link
Member

@oliviertassinari oliviertassinari Dec 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sarpt I have wondered about it too. I don't think that it's really important. I'm not aware of having faced issues with it in the past.
However, having outdated TypeScript definitions has been a common theme in the past. A migration to TypeScript would prevent us to forget about it #15984. To keep thinking about it. At the same time, only 10% of our users use the TypeScript version of the demos in our documentation (but growing), maybe the problem is not that important either. I don't know 🤔.

@mui-pr-bot
Copy link

No bundle size changes comparing 53ef743...2538f1e

Generated by 🚫 dangerJS against 2538f1e

@mbrookes mbrookes changed the title fix issue 18743 - typescript does not recognize size prop for TextField [TextField] size prop not recognized by TypeScript Dec 9, 2019
@oliviertassinari oliviertassinari added component: text field This is the name of the generic UI component, not the React module! typescript labels Dec 9, 2019
@oliviertassinari oliviertassinari changed the title [TextField] size prop not recognized by TypeScript [TextField] Fix missing size prop in TypeScript types Dec 9, 2019
@oliviertassinari oliviertassinari merged commit bd49992 into mui:master Dec 9, 2019
@oliviertassinari
Copy link
Member

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliviertassinari oliviertassinari oliviertassinari approved these changes

Assignees
No one assigned
Labels
component: text field This is the name of the generic UI component, not the React module! typescript
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[TextField] TypeScript does not recognize "size" prop
3 participants
@sarpt @mui-pr-bot @oliviertassinari