City-of-Helsinki / helsinki-design-system

Components, principles, documentation and templates for the City of Helsinki design system.
MIT License
170 stars 41 forks source link

[Combobox] Type error `TS2322` occurs - when defining multiselect Combox with pre-selected values. #329

Closed erikjuhani closed 3 years ago

erikjuhani commented 3 years ago

Describe the bug

Type error TS2322 occurs, when defining a multiselect [Combobox] with pre-selected values.

const value = [{ label: "Plutonium" }, { label: "Americium" } ];
const options = [ ...value, { label: "Copernicium" }, { label: "Nihonium" } ];
<Combobox
   multiselect
   value={value}
   options={options}
   ...
/>

I looked through the code and it seems to be caused by this line: https://github.com/City-of-Helsinki/helsinki-design-system/blob/83d5c46b01d8ad45df1626e68065dca676b15029/packages/react/src/components/dropdown/select/Select.tsx#L231

If the OptionType is defined as an array e.g.

type OptionTypeArray = { label: string }[]

this will then result as a wrong type (2D array in this case).

{ label: string }[][]

How to reproduce

  1. Implement <Combobox /> with multiselect
  2. Set value prop to contain an array of selected items
  3. Set options prop to contain all possible items including selected
  4. Build/test code && type error should now occur

Expected behavior

When creating a multiselect [Combobox] with pre-selected values (array). It should not intepret the OptionType as 2D array.

Possible solution

Change OptionType[] to OptionType. https://github.com/City-of-Helsinki/helsinki-design-system/blob/83d5c46b01d8ad45df1626e68065dca676b15029/packages/react/src/components/dropdown/select/Select.tsx#L231

aleksik commented 3 years ago

Hi, Erik!

The OptionType which is passed to the Combobox is supposed to represent the type of a single option item. So instead of passing the OptionTypeArray, you should pass a SingleOptionType.

For example:

type SingleOptionType = { label: string };

const value = [{ label: "Plutonium" }, { label: "Americium" }];
const options = [ ...value, { label: "Copernicium" }, { label: "Nihonium" }];

<Combobox<SingleOptionType>
  multiselect
  value={value}
  options={options}
  ...
/>

Here's a quick demo which hopefully clarifies this: https://codesandbox.io/s/twilight-sound-vjz2l?file=/src/App.tsx

erikjuhani commented 3 years ago

Okay thanks for clarifying this! So in the end this was just an user error for not realizing to pass the type property.