fix: regression on Select component when handling null values (#19326)

apache · Mar 23, 2022 · 9e58916 · 9e58916
1 parent b0397be
commit 9e58916
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 2 deletions.
diff --git a/superset-frontend/src/components/Select/Select.test.tsx b/superset-frontend/src/components/Select/Select.test.tsx
@@ -50,6 +50,10 @@ const OPTIONS = [
   { label: 'Cher', value: 22, gender: 'Female' },
   { label: 'Her', value: 23, gender: 'Male' },
 ].sort((option1, option2) => option1.label.localeCompare(option2.label));
+const NULL_OPTION = { label: '<NULL>', value: null } as unknown as {
+  label: string;
+  value: number;
+};
 
 const loadOptions = async (search: string, page: number, pageSize: number) => {
   const totalCount = OPTIONS.length;
@@ -384,6 +388,30 @@ test('does not add a new option if allowNewOptions is false', async () => {
   expect(await screen.findByText(NO_DATA)).toBeInTheDocument();
 });
 
+test('adds the null option when selected in single mode', async () => {
+  render(<Select {...defaultProps} options={[OPTIONS[0], NULL_OPTION]} />);
+  await open();
+  userEvent.click(await findSelectOption(NULL_OPTION.label));
+  const values = await findAllSelectValues();
+  expect(values[0]).toHaveTextContent(NULL_OPTION.label);
+});
+
+test('adds the null option when selected in multiple mode', async () => {
+  render(
+    <Select
+      {...defaultProps}
+      options={[OPTIONS[0], NULL_OPTION, OPTIONS[2]]}
+      mode="multiple"
+    />,
+  );
+  await open();
+  userEvent.click(await findSelectOption(OPTIONS[0].label));
+  userEvent.click(await findSelectOption(NULL_OPTION.label));
+  const values = await findAllSelectValues();
+  expect(values[0]).toHaveTextContent(OPTIONS[0].label);
+  expect(values[1]).toHaveTextContent(NULL_OPTION.label);
+});
+
 test('static - renders the select with default props', () => {
   render(<Select {...defaultProps} />);
   expect(getSelect()).toBeInTheDocument();

diff --git a/superset-frontend/src/components/Select/utils.ts b/superset-frontend/src/components/Select/utils.ts
@@ -25,6 +25,14 @@ import {
   GroupedOptionsType,
 } from 'react-select';
 
+function isObject(value: unknown): value is Record<string, unknown> {
+  return (
+    value !== null &&
+    typeof value === 'object' &&
+    Array.isArray(value) === false
+  );
+}
+
 /**
  * Find Option value that matches a possibly string value.
  *
@@ -63,7 +71,7 @@ export function findValue<OptionType extends OptionTypeBase>(
 export function getValue(
   option: string | number | { value: string | number | null } | null,
 ) {
-  return option && typeof option === 'object' ? option.value : option;
+  return isObject(option) ? option.value : option;
 }
 
 type LabeledValue<V> = { label?: ReactNode; value?: V };
@@ -78,7 +86,7 @@ export function hasOption<V>(
     optionsArray.find(
       x =>
         x === value ||
-        (typeof x === 'object' &&
+        (isObject(x) &&
           (('value' in x && x.value === value) ||
             (checkLabel && 'label' in x && x.label === value))),
     ) !== undefined