DefinitelyTyped / DefinitelyTyped

The repository for high quality TypeScript type definitions.
Other
48.55k stars 30.15k forks source link

react-select: Select.onChange typings wrong (?) #32553

Open JoshMcCullough opened 5 years ago

JoshMcCullough commented 5 years ago

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/6f8151b70381d83a6b7e2fc1a44086047144eb9e/types/react-select/lib/Select.d.ts#L166

I believe this should be defined as:

onChange?: (value: OptionType | OptionType[], action: ActionMeta) => void;

When an option is selected, the raw option object is sent, not the ValueType of that object.

hgezim commented 5 years ago

I found this entirely confusing.

value in the handle is the array of selected options (when using isMuli prop).

_onTagChange(
  value: ReactSelectTypes.ValueType<{value: number; label: string}>,
  action: ReactSelectTypes.ActionMeta
) {
  console.log('l68:', value, ' action:', action);
  switch(action.action) {
    case 'pop-value':
    case 'remove-value':
  }
  if (value && value.length > 0) {
   e let firstOption = value[0]; // Errors here with: Element implicitly has an 'any' type because type '{ value: number; label: string; } | { value: number; label: string; }[]' has no index signature.

  }
  // this.setState({ categoryTags: tags });
}
rvanlaarhoven commented 5 years ago

The first argument of the onChange handler cannot simply be of type OptionType, since it can be either one-, multiple- or no values of OptionType.

See https://github.com/JedWatson/react-select/issues/2902 for a more in-depth discussion about the onChange handler in combination with TypeScript.

JoshMcCullough commented 5 years ago

@rvanlaarhoven -- Updated OP to accept single or array. IMO an array should be passed to the handler regardless of whether or not the select isMulti. We should just expect an array and not have to check if the parameter is an array. But oh well...

rafaelbiten commented 4 years ago

I agree with @JoshMcCullough It would be easier if we only had to deal with a single data type, on this case, Arrays. If we're using single select, we just grab the single item from the array, else we work with the array - no extra checks needed. At the same time, I understand this is not a simple change when dealing with a library like this one.

navignaw commented 4 years ago

The biggest annoyance for me is that an array or single value is returned depending on the value of isMulti. This means that the type system should already know the type of onChange.

Could we do some sort of unioning or conditional typing to resolve this? (warning: untested ideas)

interface BaseProps<OptionType extends OptionTypeBase = { label: string; value: string }> extends SelectComponentsProps {
  ... /* all existing fields except isMulti and onChange */
}

interface SingleValueProps<...> extends BaseProps<...> {
  isMulti: false;
  onChange: (value: OptionType | null | undefined, action: ActionMeta) => void;
}

interface MultiValueProps<...> extends BaseProps<...> {
  isMulti: true;
  onChange: (value: OptionsType<OptionType> | null | undefined, action: ActionMeta) => void;
}

/* where the props are defined */
selectProps: SingleValueProps<...> | MultiValueProps<...>

or perhaps

interface Props<IsMulti extends boolean, OptionType extends OptionTypeBase = { label: string; value: string }> extends SelectComponentsProps {
  ...
  isMulti: IsMulti,
  onChange: (value: ValueType<IsMulti, OptionType>, action: ActionMeta) => void;
  ...
}

export type ValueType<IsMulti extends boolean, OptionType extends OptionTypeBase> =
  (IsMulti extends false ? OptionType : OptionsType<OptionType>) | null | undefined;

EDIT: after thinking about this some more, the type is inferrable if you hard-code isMulti (e.g. <Select isMulti={false} /> but not if you provide a statement like <Select isMulti={getValue()} />, so this strategy probably won't work.

seanlaff commented 4 years ago

How do I use the current definition without compiler errors?

Screen Shot 2020-01-10 at 4 50 20 PM

It seems the isArray check is no longer sufficient since the type is not OptionType | OptionType[] and typescript can't infer it.

rvanlaarhoven commented 4 years ago

I used a type assertion as a workaround here:

props.onChange((v as YourOptionTypeHere).value);
hoetmaaiers commented 4 years ago

Is an interface (Typescript) available for the multi value undefined, Array<Option>, Option ?

jonyyz commented 4 years ago

@hoetmaaiers You could make a helper:

type Nullable<T> = T | null | undefined;
Santiago8888 commented 4 years ago

As a workaround, I defined an interface: interface iOption { label: string; value: string; };

And then, casted the parameter: onChange={(option) => myMethod((option as iOption).value) }

Newbie012 commented 4 years ago

using as defeats the whole purpose of using typescript. Who am I supposed to get the value out of an

option: ValueType<{value :string, label: string}>

without using as? the current workaround is not a solution and may (probably will) lead to bugs in the future.

m-valvano-zakeke commented 4 years ago

I'm still having this issue and it's really annoying to do on every onChange (because I'm not using any multi select) the cast to the single option type....

Every time i must use item => (item as OptionTypeBase).value. Really bad.

pierpo commented 3 years ago

Indeed. The typing should be smart enough to determine whether it is an array (if isMulti is true) or a single value. It was working in v2 according to this thread https://github.com/JedWatson/react-select/issues/2902.

seahorsepip commented 3 years ago

Update, having to use multiple components and the above issue was bothersome.

So I combined them all into a single Select component, isMulti is now a required prop so it knows the type of value and onChange.

import ReactSelect, { ActionMeta, OptionTypeBase, Props } from 'react-select';
import AsyncSelect, { AsyncProps } from 'react-select/async';
import Creatable, { CreatableProps } from 'react-select/creatable';

// We use a generic param P here since directly omitting from a interface with generic params doesn't work
type IGenericSelectProps<OptionType, P = Props<OptionType>> =
    Omit<P, 'value' | 'onChange' | 'isMulti'>
    & Partial<AsyncProps<OptionType>>
    & Partial<CreatableProps<OptionType>>
    & {
    isMulti: boolean;
}

interface ISingleSelectProps<OptionType> extends IGenericSelectProps<OptionType> {
    value: OptionType | null | undefined;
    onChange?: (option: OptionType | null | undefined, action: ActionMeta<OptionType>) => void;
    isMulti: false;
}

interface IMultiSelectProps<OptionType> extends IGenericSelectProps<OptionType> {
    value: OptionType[];
    onChange?: (option: OptionType[], action: ActionMeta<OptionType>) => void;
    isMulti: true;
}

type ISelectProps<OptionType> = ISingleSelectProps<OptionType> | IMultiSelectProps<OptionType>;

const Select = <OptionType extends OptionTypeBase = { label: string; value: string }>(props: ISelectProps<OptionType>) => {
    // Use correct react-select component based on props
    let Component: ComponentType<ISelectProps<OptionType>> = ReactSelect as any;
    if (props.loadOptions && props.onCreateOption) {
        Component = AsyncCreatable as any;
    } else if (props.loadOptions) {
        Component = AsyncSelect as any;
    } else if (props.onCreateOption) {
        Component = Creatable as any;
    }

    return <Component {...props}/>;
};
olefrank commented 3 years ago

any news on this?

OscarJVD commented 1 year ago

any news on this?

SwannyAlves commented 5 months ago

any news on this?