[v4.0.0-rc.1] SelectInput onChange type incorrect

[vespasianvs]

What you were expecting:
The typing for the onChange handler for SelectInput is shown as:
React.ChangeEventHandler<HTMLInputElement | HTMLTextAreaElement> | undefined

This should return an event of type: React.ChangeEvent<HTMLInputElement | HTMLTextAreaAlement>

What happened instead:
A string is returned with the value that the select was changed to.

Steps to reproduce:
While using Typescript, add a SelectInput to an Edit -> SimpleForm and then add an onChange handler. Typescript marks onChange as above, but the return value doesn't match.

Related code:

• Preferably, a CodeSandbox forked from https://codesandbox.io/s/github/marmelab/react-admin/tree/master/examples/simple
• A link to a GitHub repo with the minimal codebase to reproduce the issue

const countryChanged: ChangeEventHandler<HTMLInputElement> = (ev) => {
  console.log("ev); // e.g. "UK"
  console.log(typeof ev); // string
};

<Edit>
  <SimpleForm>
	  <SelectInput
		  label="Country"
		  source="contactAddress.countryName"
		  choices={countries}
		  optionText="title"
		  optionValue="id"
		  onChange={countryChanged}
	  />
  </SimpleForm>
</Edit>

Happy to fix and do a PR, but not sure whether the typing is incorrect and a string should be returned, or whether the the typing is correct and an event should be returned.

Environment

• React-admin version: v4.0.0-rc.1
• React version: 17.0.2
• Browser: Edge v100.0.1185.36

[slax57]

Hi!

Thank you for submitting this.

Actually the full type shown by TS is: onChange: ((...event: any[]) => void) & React.ChangeEventHandler<HTMLInputElement | HTMLTextAreaElement>

But you are right when you say that in RA the argument passed to this function will always be the input's value.
Be careful though because the provided value will not always be a string. For instance the SelectArrayInput will give you an array.

In the docs we do not explicit what this function expects, and in TS I believe the type is not fundamentally wrong, although the fact that the argument is called event is confusing.
We could change it to value to make it clearer.
@fzaninotto or @WiXSL what do you think?

[vespasianvs]

The problem is that if I try:

const countryChanged: ChangeEventHandler<HTMLInputElement> = (val) => {
  /* Fails with "Conversion of type 'ChangeEvent<HTMLInputElement>' to type 'string' may be a mistake because neither type 
      sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first." */
  const countryCode = val as string;
  console.log(countryCode);
};

This does work:

const countryChanged = (...val: unknown[]) => {
	console.log(val); // Returns ["GB"] which I can they pull the string out.
}

(I use unknown[] as my company has linting for 'no-explicit-any').

Either way, if an option on typing is to use a ChangeEventHandler, it should either return a ChangeEvent or not offer that as a type IMO? Offering a ChangeEventHandler but returning an unexpected type is a recipe for problems.

[slax57]

Indeed you are totally right @vespasianvs , I missed that point.
I need to discuss with the team about this one, we'll come back to you shortly.
Labeling as potential bug for now.