[ChoiceList] Improve the type safety and correctness of ChoiceList

.changeset/old-guests-cover.md

@@ -0,0 +1,5 @@
+---
+'@shopify/polaris': patch
+---
+
+Improve the type safety and accuracy of ChoiceList

polaris-react/src/components/ChoiceList/ChoiceList.stories.tsx

@@ -1,15 +1,18 @@
 import React, {useCallback, useState} from 'react';
 import type {ComponentMeta} from '@storybook/react';
+import type {ChoiceListProps} from '@shopify/polaris';
 import {ChoiceList, TextField} from '@shopify/polaris';
 
 export default {
   component: ChoiceList,
 } as ComponentMeta<typeof ChoiceList>;
 
-export function Default() {
-  const [selected, setSelected] = useState(['hidden']);
+type HiddenOptionalRequired = 'hidden' | 'optional' | 'required';
 
-  const handleChange = useCallback((value) => setSelected(value), []);
+export function Default() {
+  const [selected, setSelected] = useState<HiddenOptionalRequired[]>([
+    'hidden',
+  ]);
 
   return (
     <ChoiceList
@@ -20,15 +23,15 @@ export function Default() {
         {label: 'Required', value: 'required'},
       ]}
       selected={selected}
-      onChange={handleChange}
+      onChange={setSelected}
     />
   );
 }
 
 export function WithError() {
-  const [selected, setSelected] = useState(['hidden']);
-
-  const handleChange = useCallback((value) => setSelected(value), []);
+  const [selected, setSelected] = useState<HiddenOptionalRequired[]>([
+    'hidden',
+  ]);
 
   return (
     <ChoiceList
@@ -39,36 +42,36 @@ export function WithError() {
         {label: 'Required', value: 'required'},
       ]}
       selected={selected}
-      onChange={handleChange}
+      onChange={setSelected}
       error="Company name cannot be hidden at this time"
     />
   );
 }
 
 export function Magic() {
-  const [selected, setSelected] = useState(['hidden']);
+  const [selected, setSelected] = useState<HiddenOptionalRequired[]>([
+    'hidden',
+  ]);
 
-  const handleChange = useCallback((value) => setSelected(value), []);
+  const choices: ChoiceListProps<HiddenOptionalRequired>['choices'] = [
+    {label: 'Hidden', value: 'hidden'},
+    {label: 'Optional', value: 'optional'},
+    {label: 'Required', value: 'required'},
+  ];
 
   return (
     <ChoiceList
       title="Company name"
-      choices={[
-        {label: 'Hidden', value: 'hidden'},
-        {label: 'Optional', value: 'optional'},
-        {label: 'Required', value: 'required'},
-      ]}
+      choices={choices}
       selected={selected}
-      onChange={handleChange}
+      onChange={setSelected}
       tone="magic"
     />
   );
 }
 
 export function WithMultiChoice() {
-  const [selected, setSelected] = useState(['hidden']);
-
-  const handleChange = useCallback((value) => setSelected(value), []);
+  const [selected, setSelected] = useState<string[]>([]);
 
   return (
     <ChoiceList
@@ -89,15 +92,13 @@ export function WithMultiChoice() {
         },
       ]}
       selected={selected}
-      onChange={handleChange}
+      onChange={setSelected}
     />
   );
 }
 
 export function MagicWithMultiChoice() {
-  const [selected, setSelected] = useState(['hidden']);
-
-  const handleChange = useCallback((value) => setSelected(value), []);
+  const [selected, setSelected] = useState<string[]>([]);
 
   return (
     <ChoiceList
@@ -118,7 +119,7 @@ export function MagicWithMultiChoice() {
         },
       ]}
       selected={selected}
-      onChange={handleChange}
+      onChange={setSelected}
       tone="magic"
     />
   );
@@ -128,24 +129,17 @@ export function WithChildrenContent() {
   const [selected, setSelected] = useState(['none']);
   const [textFieldValue, setTextFieldValue] = useState('');
 
-  const handleChoiceListChange = useCallback((value) => setSelected(value), []);
-
-  const handleTextFieldChange = useCallback(
-    (value) => setTextFieldValue(value),
-    [],
-  );
-
   const renderChildren = useCallback(
     () => (
       <TextField
         label="Minimum Quantity"
         labelHidden
-        onChange={handleTextFieldChange}
+        onChange={setTextFieldValue}
         value={textFieldValue}
         autoComplete="off"
       />
     ),
-    [handleTextFieldChange, textFieldValue],
+    [textFieldValue],
   );
 
   return (
@@ -164,7 +158,7 @@ export function WithChildrenContent() {
         },
       ]}
       selected={selected}
-      onChange={handleChoiceListChange}
+      onChange={setSelected}
     />
   );
 }
@@ -173,25 +167,18 @@ export function WithDynamicChildrenContent() {
   const [selected, setSelected] = useState(['none']);
   const [textFieldValue, setTextFieldValue] = useState('');
 
-  const handleChoiceListChange = useCallback((value) => setSelected(value), []);
-
-  const handleTextFieldChange = useCallback(
-    (value) => setTextFieldValue(value),
-    [],
-  );
-
   const renderChildren = useCallback(
     (isSelected) =>
       isSelected && (
         <TextField
           label="Minimum Quantity"
           labelHidden
-          onChange={handleTextFieldChange}
+          onChange={setTextFieldValue}
           value={textFieldValue}
           autoComplete="off"
         />
       ),
-    [handleTextFieldChange, textFieldValue],
+    [textFieldValue],
   );
 
   return (
@@ -212,7 +199,7 @@ export function WithDynamicChildrenContent() {
           },
         ]}
         selected={selected}
-        onChange={handleChoiceListChange}
+        onChange={setSelected}
       />
     </div>
   );

polaris-react/src/components/ChoiceList/ChoiceList.tsx

@@ -10,9 +10,9 @@ import {Bleed} from '../Bleed';
 
 import styles from './ChoiceList.scss';
 
-interface Choice {
+interface Choice<TValue extends string> {
   /** Value of the choice */
-  value: string;
+  value: TValue;
   /** Label for the choice */
   label: React.ReactNode;
   /** A unique identifier for the choice */
@@ -27,13 +27,13 @@ interface Choice {
   renderChildren?(isSelected: boolean): React.ReactNode | false;
 }
 
-export interface ChoiceListProps {
+export interface ChoiceListProps<TValue extends string = string> {
   /** Label for list of choices */
   title: React.ReactNode;
   /** Collection of choices */
-  choices: Choice[];
+  choices: Choice<TValue>[];
   /** Collection of selected choices */
-  selected: string[];
+  selected: TValue[];
   /** Name for form input */
   name?: string;
   /** Allow merchants to select multiple options at once */
@@ -45,12 +45,12 @@ export interface ChoiceListProps {
   /** Disable all choices **/
   disabled?: boolean;
   /** Callback when the selected choices change */
-  onChange?(selected: string[], name: string): void;
+  onChange?(selected: TValue[], name: string): void;
   /** Indicates the tone of the choice list */
   tone?: 'magic';
 }
 
-export function ChoiceList({
+export function ChoiceList<TValue extends string>({
   title,
   titleHidden,
   allowMultiple,
@@ -61,7 +61,7 @@ export function ChoiceList({
   disabled = false,
   name: nameProp,
   tone,
-}: ChoiceListProps) {
+}: ChoiceListProps<TValue>) {
   // Type asserting to any is required for TS3.2 but can be removed when we update to 3.3
   // see https://github.com/Microsoft/TypeScript/issues/28768
   const ControlComponent: any = allowMultiple ? Checkbox : RadioButton;
@@ -97,7 +97,7 @@ export function ChoiceList({
       );
     }
 
-    const isSelected = choiceIsSelected(choice, selected);
+    const isSelected = selected.includes(choice.value);
     const renderedChildren = choice.renderChildren
       ? choice.renderChildren(isSelected)
       : null;
@@ -116,7 +116,7 @@ export function ChoiceList({
             label={label}
             disabled={choiceDisabled || disabled}
             fill={{xs: true, sm: false}}
-            checked={choiceIsSelected(choice, selected)}
+            checked={isSelected}
             helpText={helpText}
             onChange={handleChange}
             ariaDescribedBy={
@@ -154,14 +154,10 @@ export function ChoiceList({
 
 function noop() {}
 
-function choiceIsSelected({value}: Choice, selected: string[]) {
-  return selected.includes(value);
-}
-
-function updateSelectedChoices(
-  {value}: Choice,
+function updateSelectedChoices<TValue extends string>(
+  {value}: Choice<TValue>,
   checked: boolean,
-  selected: string[],
+  selected: TValue[],
   allowMultiple = false,
 ) {
   if (checked) {