-
Notifications
You must be signed in to change notification settings - Fork 407
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
renderMenuItemChildren's type is too generic #704
Comments
Hey @ianatha, thanks for the report. I believe this is an issue with |
Still working on this, but it's a much larger change than I had thought. |
@ianatha I had looked into this issue a bit and found that to be the case as well, since the option type needs to be propagated to most of the code paths in the library. Thanks for sticking with this one. |
I gave this a quick test and I think it's safe to say it's probably not a good idea for the health of this codebase: https://github.com/ericgio/react-bootstrap-typeahead/compare/main...orta:react-bootstrap-typeahead:infer_objects?expand=1 - it basically needs to add the generics marker in every function/component in the system ( This mostly works, but isn't perfect ) |
Thanks for taking a crack at this, @orta. If propagating the generic type throughout the codebase isn't a feasible solution, do you have any thoughts or recommendations on how consumers of the library can best type the options being passed via props/callbacks? |
So long as your type conforms to <AsyncTypeahead
{...props}
multiple
options={users}
onChange={(users: User[]) => { ... }}
/> |
This is not possible, when you use Strict in tsconfig.json. You would get an error Type 'Option[]' is not assignable to type '{ key: string; }[]'.
Type 'Option' is not assignable to type '{ key: string; }'.
Type 'string' is not assignable to type '{ key: string; }'. |
What I have done to "fix" this issue is made a wrapper for
import {ReactElement} from 'react';
import {Typeahead as SourceTypeahead} from 'react-bootstrap-typeahead';
import {TypeaheadComponentProps} from 'react-bootstrap-typeahead/types/components/Typeahead';
import {RenderMenuItemChildren, TypeaheadMenuProps} from 'react-bootstrap-typeahead/types/components/TypeaheadMenu';
import {
FilterByCallback,
LabelKey,
RenderToken,
RenderTokenProps,
TypeaheadPropsAndState,
} from 'react-bootstrap-typeahead/types/types';
type TypedTypeaheadProps<T extends Option> = Omit<
TypeaheadComponentProps,
'filterBy' | 'renderMenuItemChildren' | 'renderToken' | 'labelKey' | 'onChange' | 'options'
> & {
filterBy?: string[] | TypedFilterByCallback<T>;
renderMenuItemChildren?: TypedRenderMenuItemChildren<T>;
renderToken?: TypedRenderToken<T>;
labelKey?: TypedLabelKey<T>;
onChange?: TypedOnChange<T>;
options: T[]
};
export const Typeahead = <T extends Option = never>(props: TypedTypeaheadProps<T>): ReactElement => {
return (
<SourceTypeahead
{...props}
filterBy={props.filterBy as string[] | FilterByCallback}
renderMenuItemChildren={props.renderMenuItemChildren as RenderMenuItemChildren}
renderToken={props.renderToken as RenderToken}
labelKey={props.labelKey as LabelKey}
onChange={props.onChange as (selected: Option[]) => void}
/>
);
};
// Importing from Typeahead messed up TS
type Option = string | Record<string, any>;
// The following wrappers are added to support generics in our callbacks. We're casting back to original later on
export type TypedLabelKey<T extends Option> = T extends object ? (string & keyof T) | ((option: T) => string) : never;
type TypedOnChange<T extends Option> = (selected: T[]) => void;
type TypedRenderToken<T extends Option> = (option: T, props: RenderTokenProps, idx: number) => JSX.Element;
type TypedFilterByCallback<T extends Option> = (option: T, state: TypeaheadPropsAndState) => boolean;
type TypedRenderMenuItemChildren<T extends Option> = (
option: T,
menuProps: TypeaheadMenuProps,
idx: number
) => JSX.Element; This way I can use import {Typeahead} from 'app/core/elements/TypedTypeahead';
type MyType = {
id: number;
name: string;
values: string[];
};
export const MyComp = () => {
return (
<Typeahead<MyType>
options={[]}
renderMenuItemChildren={(item, menuProps) => <div />}
filterBy={['name']}
onChange={(selected) => {}}
/>
);
}; |
@ericgio Is there any way I could help you with this kind of typing support? |
@HansAarneLiblik: I'm not sure, do you want to give it a shot? The main problem described in the thread is that to truly propagate the generic type basically involves passing it through the entire codebase. If you have a solution that avoids that, then it might be feasible. |
Hi I fixed the type problems with type casting: return (
<AsyncTypeahead
ref={typeahead}
filterBy={filterBy}
id='async-example'
isLoading={isLoading}
labelKey='labelDe'
className='w-50'
minLength={3}
onChange={(selected) => {
if (!selected[0]) return;
addItem((selected[0] as Item).id!);
typeahead.current!.clear();
}}
onSearch={handleSearch}
options={options}
placeholder={t('addHint', { keyPrefix: 'item' })}
//// see https://github.com/ericgio/react-bootstrap-typeahead/issues/704
renderMenuItemChildren={(option) => {
return (
<>
<span>{(option as Item).labelDe}</span>
</>
);
}}
/>
); whereas 'Item' is my model type. Maybe this might help others too. full code/project can be found here: https://github.com/mtnstar/tplanr/blob/main/frontend/src/components/Tour/Item/TypeAhead.tsx |
The
renderMenuItemChildren
property's type is(option: Option, ...) => JSX.Element
, withOption
defined asstring | Record<string, any>
. This means that implementors ofrenderMenuItemChildren
need to handle both astring
and aRecord<string, any>
(by casting onto the right type).However, the actual type of "Option" should be inferred from the type of the
options
parameters.Pull request forthcoming.
The text was updated successfully, but these errors were encountered: