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

[docs] Add some Selection-Controls TypeScript demos #15408

Merged
merged 7 commits into from
Apr 22, 2019
Merged

Conversation

bh1505
Copy link
Contributor

@bh1505 bh1505 commented Apr 18, 2019

Adding Typescript demos for some of the Selection-Controls components as a part of #14897

@mui-pr-bot
Copy link

mui-pr-bot commented Apr 18, 2019

No bundle size changes comparing a494652...6612edc

Generated by 🚫 dangerJS against 6612edc

@oliviertassinari oliviertassinari changed the title Add some Selection-Controls Typescript demos [docs] Add some Selection-Controls TypeScript demos Apr 18, 2019
@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation typescript labels Apr 18, 2019
const classes = useStyles();
const [value, setValue] = React.useState('female');

function handleChange(event: React.ChangeEvent<any>) {
Copy link
Member

Choose a reason for hiding this comment

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

Would it work? Do we need any?

Suggested change
function handleChange(event: React.ChangeEvent<any>) {
function handleChange(event: React.ChangeEvent<HTMLInputElement>) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No it does not work, I kept getting this error:

10-17-178-40:selection-controls brandonherrera$ yarn docs:typescript:check
yarn run v1.15.2
$ tslint -p docs/tsconfig.json

ERROR: /Users/brandonherrera/material-ui/docs/src/pages/demos/selection-controls/RadioButtonsGroup.tsx:39:11 - TypeScript@next compile error: 
Type '(event: ChangeEvent<HTMLInputElement>) => void' is not assignable to type '(event: ChangeEvent<{}>, value: string) => void'.
  Types of parameters 'event' and 'event' are incompatible.
    Type 'ChangeEvent<{}>' is not assignable to type 'ChangeEvent<HTMLInputElement>'.
      Type '{}' is missing the following properties from type 'HTMLInputElement': accept, align, alt, autocomplete, and 289 more.
ERROR: /Users/brandonherrera/material-ui/docs/src/pages/demos/selection-controls/RadioButtonsGroup.tsx:59:11 - TypeScript@next compile error: 
Type '(event: ChangeEvent<HTMLInputElement>) => void' is not assignable to type '(event: ChangeEvent<{}>, value: string) => void'.

error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

I used "any" because it was suggested in #13065.

Copy link
Member

Choose a reason for hiding this comment

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

Use the any cast as late as possible. Rather React.ChangeEvent<unknown> and the cast to any when you need to pass event.target.value around.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eps1lon Just to clarify, are you suggesting to change from

function handleChange(event: React.ChangeEvent<any>) {
     setValue(event.target.value);
}

to this?

function handleChange(event: React.ChangeEvent<unknown>) {
     setValue((event as any).target.value);
}

Copy link
Member

@eps1lon eps1lon Apr 22, 2019

Choose a reason for hiding this comment

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

Almost. With "as late as possible" I meant event.target.value as string (event.target as HTMLInputElement).value.

The problem is that the target type depends on the actual react context. It is a HTMLInputElement in this example but it's technically allowed to attach the handler to other element types.

Copy link
Member

Choose a reason for hiding this comment

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

it's technically allowed to attach the handler to other element types.

Is that different from the Switch or Checkbox components? What do you mean by attaching the handler to another element type?

@eps1lon eps1lon removed their assignment Apr 22, 2019
@eps1lon eps1lon mentioned this pull request Apr 22, 2019
@eps1lon eps1lon merged commit 5ac5e7f into mui:next Apr 22, 2019
@eps1lon
Copy link
Member

eps1lon commented Apr 22, 2019

@bh1505 Much appreciated. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants