From f13dbb32bbb1fd12c4bfa04b06aac54bbf4b5638 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 2 May 2025 16:36:44 -0400 Subject: [PATCH] Update dependency downshift to v9.0.9 (#103777) * Update dependency downshift to v9.0.9 * change conditional typing to error on the onChange property with an easier to read error instead of on the component add workaround for type script not being able to infer when onChange can accept null add type test * better Combobox types! * remove unused import * make type changes more backwards compatible * improve comment * even better comments --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: samsch --- .../src/components/Combobox/Combobox.test.tsx | 9 +++ .../src/components/Combobox/Combobox.tsx | 71 ++++++++++++------- .../src/components/Combobox/MultiCombobox.tsx | 4 +- yarn.lock | 6 +- 4 files changed, 59 insertions(+), 31 deletions(-) diff --git a/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx b/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx index 0c256d707bc..12b37c9f4bf 100644 --- a/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx +++ b/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx @@ -633,3 +633,12 @@ describe('Combobox', () => { }); }); }); + +// Type test +(() => { + // Handler function does not allow null for option. + function onChangeHandlerNoNull(option: ComboboxOption) {} + // @ts-expect-error with isClearable set, onChange can pass `null`, so a function that does not accept null + // is an error. If this line errors, then the conditional typing for onChange has been broken. + return ; +})(); diff --git a/packages/grafana-ui/src/components/Combobox/Combobox.tsx b/packages/grafana-ui/src/components/Combobox/Combobox.tsx index 0d0ab498727..f06ce30060c 100644 --- a/packages/grafana-ui/src/components/Combobox/Combobox.tsx +++ b/packages/grafana-ui/src/components/Combobox/Combobox.tsx @@ -20,15 +20,12 @@ import { isNewGroup } from './utils'; // TODO: It would be great if ComboboxOption["label"] was more generic so that if consumers do pass it in (for async), // then the onChange handler emits ComboboxOption with the label as non-undefined. -export interface ComboboxBaseProps + +interface ComboboxStaticProps extends Pick< InputProps, 'placeholder' | 'autoFocus' | 'id' | 'aria-labelledby' | 'disabled' | 'loading' | 'invalid' > { - /** - * An `X` appears in the UI, which clears the input and sets the value to `null`. Do not use if you have no `null` case. - */ - isClearable?: boolean; /** * Allows the user to set a value which is not in the list of options. */ @@ -40,11 +37,6 @@ export interface ComboboxBaseProps */ options: Array> | ((inputValue: string) => Promise>>); - /** - * onChange handler is called with the newly selected option. - */ - onChange: (option: ComboboxOption) => void; - /** * Current selected value. Most consumers should pass a scalar value (string | number). However, sometimes with Async * it may be better to pass in an Option with a label to display. @@ -64,18 +56,32 @@ export interface ComboboxBaseProps onBlur?: () => void; } -type ClearableConditionals = - | { - /** - * Allow the user to clear the selected value. `null` is emitted from the onChange handler - */ - isClearable: true; - /** - * The onChange handler is called with `null` when clearing the Combobox. - */ - onChange: (option: ComboboxOption | null) => void; - } - | { isClearable?: false; onChange: (option: ComboboxOption) => void }; +interface ClearableProps { + /** + * An `X` appears in the UI, which clears the input and sets the value to `null`. Do not use if you have no `null` case. + */ + isClearable: true; + + /** + * onChange handler is called with the newly selected option. + */ + onChange: (option: ComboboxOption | null) => void; +} + +interface NotClearableProps { + /** + * An `X` appears in the UI, which clears the input and sets the value to `null`. Do not use if you have no `null` case. + */ + isClearable?: false; + + /** + * onChange handler is called with the newly selected option. + */ + onChange: (option: ComboboxOption) => void; +} + +export type ComboboxBaseProps = (ClearableProps | NotClearableProps) & + ComboboxStaticProps; export type AutoSizeConditionals = | { @@ -95,9 +101,7 @@ export type AutoSizeConditionals = maxWidth?: never; }; -export type ComboboxProps = ComboboxBaseProps & - AutoSizeConditionals & - ClearableConditionals; +export type ComboboxProps = ComboboxBaseProps & AutoSizeConditionals; const noop = () => {}; @@ -114,7 +118,7 @@ export const Combobox = (props: ComboboxProps) => onChange, value: valueProp, placeholder: placeholderProp, - isClearable = false, + isClearable, // this should be default false, but TS can't infer the conditional type if you do createCustomValue = false, id, width, @@ -244,7 +248,20 @@ export const Combobox = (props: ComboboxProps) => // Instead, stateReducer is called in the same tick as state changes, before that state is committed and rendered. onSelectedItemChange: ({ selectedItem }) => { - onChange(selectedItem); + // `selectedItem` type is `ComboboxOption | null` + // It can be null when `selectItem()` is called with null, and we never do that unless `isClearable` is true. + // So, when `isClearable` is false, `selectedItem` is always non-null. However, the types don't reflect that, + // which is why the conditions are needed. + // + // this is an else if because TS can't infer the correct onChange types from + // (isClearable || selectedItem !== null) + if (isClearable) { + // onChange argument type allows null + onChange(selectedItem); + } else if (selectedItem !== null) { + // onChange argument type *does not* allow null + onChange(selectedItem); + } }, defaultHighlightedIndex: selectedItemIndex ?? 0, diff --git a/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx b/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx index 56a24fd8eef..5fb513f0d7b 100644 --- a/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx +++ b/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx @@ -23,9 +23,11 @@ import { MAX_SHOWN_ITEMS, useMeasureMulti } from './useMeasureMulti'; import { useMultiInputAutoSize } from './useMultiInputAutoSize'; import { useOptions } from './useOptions'; -interface MultiComboboxBaseProps extends Omit, 'value' | 'onChange'> { +interface MultiComboboxBaseProps + extends Omit, 'value' | 'onChange' | 'isClearable'> { value?: T[] | Array>; onChange: (option: Array>) => void; + isClearable?: boolean; enableAllOption?: boolean; } diff --git a/yarn.lock b/yarn.lock index cbbffcb3ec9..4e50953f3ed 100644 --- a/yarn.lock +++ b/yarn.lock @@ -14976,8 +14976,8 @@ __metadata: linkType: hard "downshift@npm:^9.0.6": - version: 9.0.8 - resolution: "downshift@npm:9.0.8" + version: 9.0.9 + resolution: "downshift@npm:9.0.9" dependencies: "@babel/runtime": "npm:^7.24.5" compute-scroll-into-view: "npm:^3.1.0" @@ -14986,7 +14986,7 @@ __metadata: tslib: "npm:^2.6.2" peerDependencies: react: ">=16.12.0" - checksum: 10/9dc4577e780c54742ba4dde11f481f0d839f001b309200fbe4db112385b227ccd9cd2ef97d9e995379fa70249f0664a562240e415b9966f18c8a5cb7ce435f2c + checksum: 10/6abc7a585f002f0ebaba1ec42f6102b940257f294d0a33bf189387a316afa24254f0d49c4d5d2553a2263b4e42f497297a8c66dab9bebe24b3dce11fa1456d20 languageName: node linkType: hard