From 2ca2f43beee9162ec0d2192836a126482e8f37c5 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Tue, 30 Jul 2024 04:23:31 +0900 Subject: [PATCH 1/6] SelectControl: Infer `value` type from `options` --- .../components/src/date-time/time/index.tsx | 35 ++++--- .../components/src/query-controls/index.tsx | 6 +- .../components/src/select-control/index.tsx | 20 +++- .../components/src/select-control/types.ts | 98 ++++++++++--------- 4 files changed, 90 insertions(+), 69 deletions(-) diff --git a/packages/components/src/date-time/time/index.tsx b/packages/components/src/date-time/time/index.tsx index e2877a1ded5e1f..072ff7fd84d096 100644 --- a/packages/components/src/date-time/time/index.tsx +++ b/packages/components/src/date-time/time/index.tsx @@ -77,10 +77,28 @@ export function TimePicker( { ); }, [ currentTime ] ); + const monthOptions = [ + { value: '01', label: __( 'January' ) }, + { value: '02', label: __( 'February' ) }, + { value: '03', label: __( 'March' ) }, + { value: '04', label: __( 'April' ) }, + { value: '05', label: __( 'May' ) }, + { value: '06', label: __( 'June' ) }, + { value: '07', label: __( 'July' ) }, + { value: '08', label: __( 'August' ) }, + { value: '09', label: __( 'September' ) }, + { value: '10', label: __( 'October' ) }, + { value: '11', label: __( 'November' ) }, + { value: '12', label: __( 'December' ) }, + ] as const; + const { day, month, year, minutes, hours } = useMemo( () => ( { day: format( date, 'dd' ), - month: format( date, 'MM' ), + month: format( + date, + 'MM' + ) as ( typeof monthOptions )[ number ][ 'value' ], year: format( date, 'yyyy' ), minutes: format( date, 'mm' ), hours: format( date, 'HH' ), @@ -146,20 +164,7 @@ export function TimePicker( { __next40pxDefaultSize __nextHasNoMarginBottom value={ month } - options={ [ - { value: '01', label: __( 'January' ) }, - { value: '02', label: __( 'February' ) }, - { value: '03', label: __( 'March' ) }, - { value: '04', label: __( 'April' ) }, - { value: '05', label: __( 'May' ) }, - { value: '06', label: __( 'June' ) }, - { value: '07', label: __( 'July' ) }, - { value: '08', label: __( 'August' ) }, - { value: '09', label: __( 'September' ) }, - { value: '10', label: __( 'October' ) }, - { value: '11', label: __( 'November' ) }, - { value: '12', label: __( 'December' ) }, - ] } + options={ monthOptions } onChange={ ( value ) => { const newDate = setMonth( date, Number( value ) - 1 ); setDate( newDate ); diff --git a/packages/components/src/query-controls/index.tsx b/packages/components/src/query-controls/index.tsx index de53c63a9b8a82..3557335ebac5a0 100644 --- a/packages/components/src/query-controls/index.tsx +++ b/packages/components/src/query-controls/index.tsx @@ -85,7 +85,11 @@ export function QueryControls( { __next40pxDefaultSize={ __next40pxDefaultSize } key="query-controls-order-select" label={ __( 'Order by' ) } - value={ `${ orderBy }/${ order }` } + value={ + orderBy === undefined || order === undefined + ? undefined + : `${ orderBy }/${ order }` + } options={ [ { label: __( 'Newest to oldest' ), diff --git a/packages/components/src/select-control/index.tsx b/packages/components/src/select-control/index.tsx index 5db0316d02b129..0ee4ad32d48146 100644 --- a/packages/components/src/select-control/index.tsx +++ b/packages/components/src/select-control/index.tsx @@ -26,8 +26,8 @@ function useUniqueId( idProp?: string ) { return idProp || id; } -function UnforwardedSelectControl( - props: WordPressComponentProps< SelectControlProps, 'select', false >, +function UnforwardedSelectControl< T extends string >( + props: WordPressComponentProps< SelectControlProps< T >, 'select', false >, ref: React.ForwardedRef< HTMLSelectElement > ) { const { @@ -66,12 +66,14 @@ function UnforwardedSelectControl( const selectedOptions = Array.from( event.target.options ).filter( ( { selected } ) => selected ); - const newValues = selectedOptions.map( ( { value } ) => value ); + const newValues = selectedOptions.map( + ( { value } ) => value as T + ); props.onChange?.( newValues, { event } ); return; } - props.onChange?.( event.target.value, { event } ); + props.onChange?.( event.target.value as T, { event } ); }; const classes = clsx( 'components-select-control', className ); @@ -164,6 +166,14 @@ function UnforwardedSelectControl( * }; * ``` */ -export const SelectControl = forwardRef( UnforwardedSelectControl ); +export const SelectControl = forwardRef( UnforwardedSelectControl ) as < + T extends string, +>( + props: WordPressComponentProps< + SelectControlProps< T >, + 'select', + false + > & { ref?: React.Ref< HTMLSelectElement > } +) => ReturnType< typeof UnforwardedSelectControl >; export default SelectControl; diff --git a/packages/components/src/select-control/types.ts b/packages/components/src/select-control/types.ts index a1d02d444c1474..441d209030d6b5 100644 --- a/packages/components/src/select-control/types.ts +++ b/packages/components/src/select-control/types.ts @@ -9,7 +9,7 @@ import type { ChangeEvent, FocusEvent, ReactNode } from 'react'; import type { InputBaseProps } from '../input-control/types'; import type { BaseControlProps } from '../base-control/types'; -type SelectControlBaseProps = Pick< +type SelectControlBaseProps< T extends string > = Pick< InputBaseProps, | '__next36pxDefaultSize' | '__next40pxDefaultSize' @@ -24,7 +24,7 @@ type SelectControlBaseProps = Pick< Pick< BaseControlProps, 'help' | '__nextHasNoMarginBottom' > & { onBlur?: ( event: FocusEvent< HTMLSelectElement > ) => void; onFocus?: ( event: FocusEvent< HTMLSelectElement > ) => void; - options?: { + options?: readonly { /** * The label to be shown to the user. */ @@ -33,7 +33,7 @@ type SelectControlBaseProps = Pick< * The internal value used to choose the selected value. * This is also the value passed to `onChange` when the option is selected. */ - value: string; + value: T; id?: string; /** * Whether or not the option should have the disabled attribute. @@ -61,50 +61,52 @@ type SelectControlBaseProps = Pick< variant?: 'default' | 'minimal'; }; -export type SelectControlSingleSelectionProps = SelectControlBaseProps & { - /** - * If this property is added, multiple values can be selected. The `value` passed should be an array. - * - * In most cases, it is preferable to use the `FormTokenField` or `CheckboxControl` components instead. - * - * @default false - */ - multiple?: false; - value?: string; - /** - * A function that receives the value of the new option that is being selected as input. - * - * If `multiple` is `true`, the value received is an array of the selected value. - * Otherwise, the value received is a single value with the new selected value. - */ - onChange?: ( - value: string, - extra?: { event?: ChangeEvent< HTMLSelectElement > } - ) => void; -}; +export type SelectControlSingleSelectionProps< T extends string = string > = + SelectControlBaseProps< T > & { + /** + * If this property is added, multiple values can be selected. The `value` passed should be an array. + * + * In most cases, it is preferable to use the `FormTokenField` or `CheckboxControl` components instead. + * + * @default false + */ + multiple?: false; + value?: NoInfer< T >; + /** + * A function that receives the value of the new option that is being selected as input. + * + * If `multiple` is `true`, the value received is an array of the selected value. + * Otherwise, the value received is a single value with the new selected value. + */ + onChange?: ( + value: T, + extra?: { event?: ChangeEvent< HTMLSelectElement > } + ) => void; + }; -export type SelectControlMultipleSelectionProps = SelectControlBaseProps & { - /** - * If this property is added, multiple values can be selected. The `value` passed should be an array. - * - * In most cases, it is preferable to use the `FormTokenField` or `CheckboxControl` components instead. - * - * @default false - */ - multiple: true; - value?: string[]; - /** - * A function that receives the value of the new option that is being selected as input. - * - * If `multiple` is `true`, the value received is an array of the selected value. - * Otherwise, the value received is a single value with the new selected value. - */ - onChange?: ( - value: string[], - extra?: { event?: ChangeEvent< HTMLSelectElement > } - ) => void; -}; +export type SelectControlMultipleSelectionProps< T extends string > = + SelectControlBaseProps< T > & { + /** + * If this property is added, multiple values can be selected. The `value` passed should be an array. + * + * In most cases, it is preferable to use the `FormTokenField` or `CheckboxControl` components instead. + * + * @default false + */ + multiple: true; + value?: NoInfer< T >[]; + /** + * A function that receives the value of the new option that is being selected as input. + * + * If `multiple` is `true`, the value received is an array of the selected value. + * Otherwise, the value received is a single value with the new selected value. + */ + onChange?: ( + value: T[], + extra?: { event?: ChangeEvent< HTMLSelectElement > } + ) => void; + }; -export type SelectControlProps = - | SelectControlSingleSelectionProps - | SelectControlMultipleSelectionProps; +export type SelectControlProps< T extends string = string > = + | SelectControlSingleSelectionProps< T > + | SelectControlMultipleSelectionProps< T >; From 34ca3a08f2cbd6b7981251bc494ee3be0ee4960d Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Fri, 2 Aug 2024 03:24:32 +0900 Subject: [PATCH 2/6] Don't infer onChange --- packages/components/src/select-control/types.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/components/src/select-control/types.ts b/packages/components/src/select-control/types.ts index 441d209030d6b5..5bf438f6490444 100644 --- a/packages/components/src/select-control/types.ts +++ b/packages/components/src/select-control/types.ts @@ -79,7 +79,7 @@ export type SelectControlSingleSelectionProps< T extends string = string > = * Otherwise, the value received is a single value with the new selected value. */ onChange?: ( - value: T, + value: NoInfer< T >, extra?: { event?: ChangeEvent< HTMLSelectElement > } ) => void; }; @@ -102,7 +102,7 @@ export type SelectControlMultipleSelectionProps< T extends string > = * Otherwise, the value received is a single value with the new selected value. */ onChange?: ( - value: T[], + value: NoInfer< T >[], extra?: { event?: ChangeEvent< HTMLSelectElement > } ) => void; }; From ad2000404f1d62b287d7b8804fedf94e8cda7361 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Fri, 2 Aug 2024 03:26:14 +0900 Subject: [PATCH 3/6] Add static type tests Co-authored-by: Mikey Binns --- .../select-control/test/select-control.tsx | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) diff --git a/packages/components/src/select-control/test/select-control.tsx b/packages/components/src/select-control/test/select-control.tsx index 03e3acbf1f7325..5b0907d7ca9f48 100644 --- a/packages/components/src/select-control/test/select-control.tsx +++ b/packages/components/src/select-control/test/select-control.tsx @@ -81,4 +81,129 @@ describe( 'SelectControl', () => { expect.anything() ); } ); + + /* eslint-disable jest/expect-expect */ + describe( 'static typing', () => { + describe( 'single', () => { + it( 'should infer the value type from available `options`, but not the `value` or `onChange` prop', () => { + const onChange: ( value: 'foo' | 'bar' ) => void = () => {}; + + ; + + value === 'string' } + />; + } ); + + it( 'should accept an explicit type argument', () => { + + // @ts-expect-error "string" is not "narrow" or "value" + value="string" + options={ [ + { + value: 'narrow', + label: 'Narrow', + }, + { + // @ts-expect-error "string" is not "narrow" or "value" + value: 'string', + label: 'String', + }, + ] } + />; + } ); + } ); + + describe( 'multiple', () => { + it( 'should infer the value type from available `options`, but not the `value` or `onChange` prop', () => { + const onChange: ( + value: ( 'foo' | 'bar' )[] + ) => void = () => {}; + + ; + + + // @ts-expect-error "string" is not "narrow" or "value" + value.forEach( ( v ) => v === 'string' ) + } + />; + } ); + + it( 'should accept an explicit type argument', () => { + + multiple + // @ts-expect-error "string" is not "narrow" or "value" + value={ [ 'string' ] } + options={ [ + { + value: 'narrow', + label: 'Narrow', + }, + { + // @ts-expect-error "string" is not "narrow" or "value" + value: 'string', + label: 'String', + }, + ] } + />; + } ); + } ); + } ); + /* eslint-enable jest/expect-expect */ } ); From 2ff5b37b9ca7827d381439e1ef2d46c9a78a3dcf Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Fri, 2 Aug 2024 03:33:49 +0900 Subject: [PATCH 4/6] Rename generic to `V` (for `value`) --- .../components/src/select-control/index.tsx | 12 ++++----- .../components/src/select-control/types.ts | 26 +++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/packages/components/src/select-control/index.tsx b/packages/components/src/select-control/index.tsx index 0ee4ad32d48146..bde02b45894846 100644 --- a/packages/components/src/select-control/index.tsx +++ b/packages/components/src/select-control/index.tsx @@ -26,8 +26,8 @@ function useUniqueId( idProp?: string ) { return idProp || id; } -function UnforwardedSelectControl< T extends string >( - props: WordPressComponentProps< SelectControlProps< T >, 'select', false >, +function UnforwardedSelectControl< V extends string >( + props: WordPressComponentProps< SelectControlProps< V >, 'select', false >, ref: React.ForwardedRef< HTMLSelectElement > ) { const { @@ -67,13 +67,13 @@ function UnforwardedSelectControl< T extends string >( ( { selected } ) => selected ); const newValues = selectedOptions.map( - ( { value } ) => value as T + ( { value } ) => value as V ); props.onChange?.( newValues, { event } ); return; } - props.onChange?.( event.target.value as T, { event } ); + props.onChange?.( event.target.value as V, { event } ); }; const classes = clsx( 'components-select-control', className ); @@ -167,10 +167,10 @@ function UnforwardedSelectControl< T extends string >( * ``` */ export const SelectControl = forwardRef( UnforwardedSelectControl ) as < - T extends string, + V extends string, >( props: WordPressComponentProps< - SelectControlProps< T >, + SelectControlProps< V >, 'select', false > & { ref?: React.Ref< HTMLSelectElement > } diff --git a/packages/components/src/select-control/types.ts b/packages/components/src/select-control/types.ts index 5bf438f6490444..84b01b75b3f576 100644 --- a/packages/components/src/select-control/types.ts +++ b/packages/components/src/select-control/types.ts @@ -9,7 +9,7 @@ import type { ChangeEvent, FocusEvent, ReactNode } from 'react'; import type { InputBaseProps } from '../input-control/types'; import type { BaseControlProps } from '../base-control/types'; -type SelectControlBaseProps< T extends string > = Pick< +type SelectControlBaseProps< V extends string > = Pick< InputBaseProps, | '__next36pxDefaultSize' | '__next40pxDefaultSize' @@ -33,7 +33,7 @@ type SelectControlBaseProps< T extends string > = Pick< * The internal value used to choose the selected value. * This is also the value passed to `onChange` when the option is selected. */ - value: T; + value: V; id?: string; /** * Whether or not the option should have the disabled attribute. @@ -61,8 +61,8 @@ type SelectControlBaseProps< T extends string > = Pick< variant?: 'default' | 'minimal'; }; -export type SelectControlSingleSelectionProps< T extends string = string > = - SelectControlBaseProps< T > & { +export type SelectControlSingleSelectionProps< V extends string = string > = + SelectControlBaseProps< V > & { /** * If this property is added, multiple values can be selected. The `value` passed should be an array. * @@ -71,7 +71,7 @@ export type SelectControlSingleSelectionProps< T extends string = string > = * @default false */ multiple?: false; - value?: NoInfer< T >; + value?: NoInfer< V >; /** * A function that receives the value of the new option that is being selected as input. * @@ -79,13 +79,13 @@ export type SelectControlSingleSelectionProps< T extends string = string > = * Otherwise, the value received is a single value with the new selected value. */ onChange?: ( - value: NoInfer< T >, + value: NoInfer< V >, extra?: { event?: ChangeEvent< HTMLSelectElement > } ) => void; }; -export type SelectControlMultipleSelectionProps< T extends string > = - SelectControlBaseProps< T > & { +export type SelectControlMultipleSelectionProps< V extends string > = + SelectControlBaseProps< V > & { /** * If this property is added, multiple values can be selected. The `value` passed should be an array. * @@ -94,7 +94,7 @@ export type SelectControlMultipleSelectionProps< T extends string > = * @default false */ multiple: true; - value?: NoInfer< T >[]; + value?: NoInfer< V >[]; /** * A function that receives the value of the new option that is being selected as input. * @@ -102,11 +102,11 @@ export type SelectControlMultipleSelectionProps< T extends string > = * Otherwise, the value received is a single value with the new selected value. */ onChange?: ( - value: NoInfer< T >[], + value: NoInfer< V >[], extra?: { event?: ChangeEvent< HTMLSelectElement > } ) => void; }; -export type SelectControlProps< T extends string = string > = - | SelectControlSingleSelectionProps< T > - | SelectControlMultipleSelectionProps< T >; +export type SelectControlProps< V extends string = string > = + | SelectControlSingleSelectionProps< V > + | SelectControlMultipleSelectionProps< V >; From 0770b1458723a8cae8933286ce8f1730cc4aff44 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Fri, 2 Aug 2024 04:26:19 +0900 Subject: [PATCH 5/6] Add changelog --- packages/components/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index e77ae39ddb3d20..b3bf6ea61a266d 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -12,6 +12,7 @@ - `TimeInput`: Expose as subcomponent of `TimePicker` ([#63145](https://github.com/WordPress/gutenberg/pull/63145)). - `RadioControl`: add support for option help text ([#63751](https://github.com/WordPress/gutenberg/pull/63751)). +- `SelectControl`: Infer `value` type from `options` ([#64069](https://github.com/WordPress/gutenberg/pull/64069)). ### Internal From 2f195c27ecf036e1f7359323fbaf87e53478071c Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Wed, 7 Aug 2024 06:16:37 +0900 Subject: [PATCH 6/6] Simplify return type --- packages/components/src/select-control/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/select-control/index.tsx b/packages/components/src/select-control/index.tsx index bde02b45894846..ddd0f23b22cc6c 100644 --- a/packages/components/src/select-control/index.tsx +++ b/packages/components/src/select-control/index.tsx @@ -174,6 +174,6 @@ export const SelectControl = forwardRef( UnforwardedSelectControl ) as < 'select', false > & { ref?: React.Ref< HTMLSelectElement > } -) => ReturnType< typeof UnforwardedSelectControl >; +) => React.JSX.Element | null; export default SelectControl;