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

[Select] onChange event Typescript typing is incorrect #10682

Closed
1 task done
thupi opened this issue Mar 16, 2018 · 4 comments
Closed
1 task done

[Select] onChange event Typescript typing is incorrect #10682

thupi opened this issue Mar 16, 2018 · 4 comments

Comments

@thupi
Copy link

thupi commented Mar 16, 2018

  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

The Typescript type for onChange on Select should be string[]. Also the docs says that onChange takes two arguments

Current Behavior

The Typescript type for onChange on Select is string and the typing allows only for one argument.

Steps to Reproduce (for bugs)

Im not quite sure how to reproduce typing errors using codesandbox. Please let me know if a reproduction repo is needed. If so i will quickly create a repo for that.

  1. Use Select component in a Typescript project(in my case typescript@2.7.2)
  2. Pass in (event, two) => { const value = event.target.value; value.join(','); console.log(two); }

Due to the usage of React.ChangeEventHandler<HTMLTextAreaElement | HTMLInputElement> for the onChange prop the type and behavior is not identical.

Context

I am trying to create a generic MultiSelect component using the Select component.

Your Environment

Tech Version
Material-UI 1.0.0-beta.37
React 16.2.0
Typescript 2.7.2
@thupi thupi changed the title [Select] onChange Typescript event type is incorrect [Select] onChange event Typescript typing is incorrect Mar 16, 2018
@t49tran
Copy link
Contributor

t49tran commented Apr 13, 2018

Hi @oliviertassinari, I think we can update SelectInput onChange type definition then make Select onChange to follow that of SelectInput without affecting InputProps definition.

Personally, I think InputProps.onChange definition at the moment is pretty sound.

export interface SelectInputProps extends StandardProps<{}, SelectInputClassKey> {
  ...
  onChange?: (event: React.ChangeEvent<HTMLSelectElement>, child: React.ReactNode) => void;
  ...
}
export interface SelectProps extends StandardProps<InputProps, SelectClassKey, 'value' | 'onChange'> {
 ...
  onChange: SelectInputProps['onChange'];
  ...
}

What do you think ?

@oliviertassinari
Copy link
Member

@t49tran This sounds good to me. Do you want to submit a pull-request? :)

@t49tran
Copy link
Contributor

t49tran commented Apr 13, 2018

Yes, I just created one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants