carbon icon indicating copy to clipboard operation
carbon copied to clipboard

MultiSelect: onChange prop is now expected to be ChangeEvent<HTMLInputElement>

Open Arkalex opened this issue 3 years ago • 1 comments

Current behaviour

Until now, every onChange was expecting a ChangeEvent<HTMLSelectElement>, but now this component is returning an error saying: ChangeEvent<HTMLSelectElement> is not assignable to type ChangeEvent<HTMLInputElement>

Expected behaviour

Keep receiving a ChangeEvent<HTMLSelectElement> on the onChange

CodeSandbox or Storybook URL

https://codesandbox.io/s/quirky-fog-ype8mv

JIRA Ticket (Sage Only)

SBS-41965

Suggested Solution

No response

Carbon Version

110.4.1

Design Tokens Version

No response

What browsers are you seeing the problem on?

Chrome

What Operating System are you seeing the problem on?

MacOS

Anything else we should know?

No response

Confidentiality

  • [X] I confirm there is no confidential or commercially sensitive information included.

Arkalex avatar Sep 14 '22 11:09 Arkalex

FE-5376

nicktitchmarsh avatar Sep 20 '22 13:09 nicktitchmarsh

Hi @Arkalex, I'm wondering if you can give me a slightly more realistic example of the intended use case here, for why you need the argument to be of type ChangeEvent<HTMLSelectElement>. (This might seem a daft question but the Select input isn't actually an HTML select element.)

So your example has made me realise that the type that we have in our definition file for onChange is wrong - and has been for a long time, the only reason your example worked before is that there was an error exporting a type that in effect made the whole MultiSelectProps interface not work, and therefore accept absolutely any props of any types at all! This was fixed (as part of another refactor) in version 110.2.1, but unfortunately this exposed the fact that the type we defined for onChange is wrong, which is why you noticed a problem.

I'll fix this, and get the type to be correct - but the correct type as I said doesn't accept a ChangeEvent<HTMLSelectElement> as argument, in fact the event handler needs to accept a custom event type, shown below:

export type CustomEvent = {
  target: {
    value: string[] | Record<string, unknown>[];
    id: string;
    name: string;
  }
};

onChange?: (ev: CustomEvent) => void;

This should be the correct type, but even after making this change your codesandbox example will still fail to compile. But if you were using any other properties on the event then the function simply wouldn't work as expected anyway, so I'm hoping it will be easy for you to rewrite your code to work with this? (In particular you can work with event.target.value as an array which you definitely couldn't get with the previous incorrect type.) Please let me know how this will affect you, as I won't make any changes until I know this works for you. Thanks!

robinzigmond avatar Oct 11 '22 13:10 robinzigmond

Hi @robinzigmond, sorry for the late response. As previously it was working fine with the type ChangeEvent<HTMLSelectElement> I thought that replacing that for a ChangeEvent<HTMLInputElement>. But with all this information, everything should be fine now

Arkalex avatar Oct 19 '22 11:10 Arkalex