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

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

Open
JoshMcCullough opened this issue Jan 28, 2019 · 16 comments
Open

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

JoshMcCullough opened this issue Jan 28, 2019 · 16 comments

Comments

@JoshMcCullough
Copy link
Contributor

JoshMcCullough commented Jan 28, 2019

onChange?: (value: ValueType<OptionType>, action: ActionMeta) => void;

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
Copy link
Contributor

hgezim commented Feb 27, 2019

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
Copy link

rvanlaarhoven commented Nov 8, 2019

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 JedWatson/react-select#2902 for a more in-depth discussion about the onChange handler in combination with TypeScript.

@JoshMcCullough
Copy link
Contributor Author

@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
Copy link

rafaelbiten commented Nov 28, 2019

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
Copy link

navignaw commented Dec 18, 2019

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
Copy link

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
Copy link

I used a type assertion as a workaround here:

props.onChange((v as YourOptionTypeHere).value);

@hoetmaaiers
Copy link

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

@jonyyz
Copy link

jonyyz commented Mar 4, 2020

@hoetmaaiers You could make a helper:

type Nullable<T> = T | null | undefined;

@Santiago8888
Copy link

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
Copy link

Newbie012 commented Oct 18, 2020

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
Copy link

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
Copy link

pierpo commented Nov 23, 2020

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 JedWatson/react-select#2902.

@seahorsepip
Copy link

seahorsepip commented Apr 7, 2021

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
Copy link
Contributor

any news on this?

1 similar comment
@OscarJVD
Copy link

any news on this?

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

No branches or pull requests