Skip to content
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.

Sign up for GitHub

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

fix: select: adds fall back and optional chaining to avoid null reference errors at runtime #669

Merged
merged 3 commits into from
Jul 20, 2023
Merged

fix: select: adds fall back and optional chaining to avoid null reference errors at runtime #669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
acde9a0
fix: select: adds optional chaining to avoid null reference errors at…
dkilgore-eightfold Jul 17, 2023
a1ab3e1
Merge branch 'main' into dkilgore-eightfold/select-null-props-fixup
dkilgore-eightfold Jul 19, 2023
feb2ae6
chore: select: fall back to empty array rather than use optional chain
dkilgore-eightfold Jul 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/components/Select/Select.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,46 @@ describe('Select', () => {
expect(container).toMatchSnapshot();
});

test('Renders null options in single mode without crashing', () => {
const { container, getAllByPlaceholderText } = render(
<Select options={null} placeholder="Select single test" />
);
const select = getAllByPlaceholderText('Select single test');
expect(() => container).not.toThrowError();
expect(select).toBeTruthy();
expect(container).toMatchSnapshot();
});

test('Renders null options in multiple mode without crashing', () => {
const { container, getAllByPlaceholderText } = render(
<Select multiple options={null} placeholder="Select multiple test" />
);
const selectMulti = getAllByPlaceholderText('Select multiple test');
expect(() => container).not.toThrowError();
expect(selectMulti).toBeTruthy();
expect(container).toMatchSnapshot();
});

test('Renders empty options in single mode without crashing', () => {
const { container, getAllByPlaceholderText } = render(
<Select options={[]} placeholder="Select single test" />
);
const select = getAllByPlaceholderText('Select single test');
expect(() => container).not.toThrowError();
expect(select).toBeTruthy();
expect(container).toMatchSnapshot();
});

test('Renders empty options in multiple mode without crashing', () => {
const { container, getAllByPlaceholderText } = render(
<Select multiple options={[]} placeholder="Select multiple test" />
);
const selectMulti = getAllByPlaceholderText('Select multiple test');
expect(() => container).not.toThrowError();
expect(selectMulti).toBeTruthy();
expect(container).toMatchSnapshot();
});

test('renders as not read-only and is editable', () => {
const { container } = render(<Select options={options} />);
expect(
Expand Down
56 changes: 28 additions & 28 deletions src/components/Select/Select.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ export const Select: FC<SelectProps> = React.forwardRef(
);
const [dropdownVisible, setDropdownVisibility] = useState<boolean>(false);
const [options, setOptions] = useState<SelectOption[]>(
_options.map((option: SelectOption, index: number) => ({
(_options || []).map((option: SelectOption, index: number) => ({
selected: false,
hideOption: false,
id: option.text + '-' + index,
Expand Down Expand Up @@ -146,13 +146,13 @@ export const Select: FC<SelectProps> = React.forwardRef(
const prevDropdownVisible: boolean = usePreviousState(dropdownVisible);

const getSelectedOptionValues = (): SelectOption['value'][] => {
return options
return (options || [])
.filter((option: SelectOption) => option.selected)
.map((option: SelectOption) => option.value);
};

const getSelectedOptions = (): SelectOption['value'][] => {
return options.filter((option: SelectOption) => option.selected);
return (options || []).filter((option: SelectOption) => option.selected);
};

const { count, filled, width } = useMaxVisibleSections(
Expand All @@ -166,14 +166,14 @@ export const Select: FC<SelectProps> = React.forwardRef(

// Populate options on component render
useEffect(() => {
const selected: SelectOption[] = options.filter(
const selected: SelectOption[] = (options || []).filter(
(opt: SelectOption) => opt.selected
);
setOptions(
_options.map((option: SelectOption, index: number) => ({
(_options || []).map((option: SelectOption, index: number) => ({
selected: !!selected.find((opt) => opt.value === option.value),
hideOption: false,
id: option.text + index,
id: option.text + '-' + index,
object: option.object,
role: 'option',
...option,
Expand All @@ -183,16 +183,16 @@ export const Select: FC<SelectProps> = React.forwardRef(

// Populate options on isLoading change
useEffect(() => {
const selected: SelectOption[] = options.filter(
const selected: SelectOption[] = (options || []).filter(
(opt: SelectOption) => opt.selected
);
setOptions(
_options.map((option: SelectOption, index: number) => ({
(_options || []).map((option: SelectOption, index: number) => ({
selected:
!!selected.find((opt) => opt.value === option.value) ||
option.value === defaultValue,
hideOption: false,
id: option.text + index,
id: option.text + '-' + index,
object: option.object,
role: 'option',
...option,
Expand Down Expand Up @@ -223,7 +223,7 @@ export const Select: FC<SelectProps> = React.forwardRef(
}, [getSelectedOptionValues().join('')]);

useEffect(() => {
const updatedOptions = options.map((opt) => ({
const updatedOptions = (options || []).map((opt) => ({
...opt,
selected:
(defaultValue !== undefined &&
Expand Down Expand Up @@ -264,7 +264,7 @@ export const Select: FC<SelectProps> = React.forwardRef(
const toggleOption = (option: SelectOption): void => {
setSearchQuery('');

const updatedOptions = options.map((opt: SelectOption) => {
const updatedOptions = (options || []).map((opt: SelectOption) => {
const defaultState: boolean = multiple ? opt.selected : false;
const selected: boolean =
opt.value === option.value ? !opt.selected : defaultState;
Expand All @@ -288,15 +288,15 @@ export const Select: FC<SelectProps> = React.forwardRef(
const resetSelectOnDropdownHide = (): void => {
setSearchQuery('');
setOptions(
options.map((opt: SelectOption) => ({
(options || []).map((opt: SelectOption) => ({
...opt,
hideOption: false,
}))
);
};

const resetSingleSelectOnDropdownToggle = (): void => {
const selected: SelectOption[] = options.filter(
const selected: SelectOption[] = (options || []).filter(
(opt: SelectOption) => opt.selected
);
if (selected.length && inputRef.current?.value !== selectedOptionText) {
Expand All @@ -308,22 +308,22 @@ export const Select: FC<SelectProps> = React.forwardRef(
setSearchQuery('');
if (filterable && multiple) {
setOptions(
options.map((opt: SelectOption) => ({
(options || []).map((opt: SelectOption) => ({
...opt,
hideOption: false,
}))
);
} else if (filterable) {
setOptions(
options.map((opt: SelectOption) => ({
(options || []).map((opt: SelectOption) => ({
...opt,
hideOption: false,
selected: false,
}))
);
} else {
setOptions(
options.map((opt: SelectOption) => ({
(options || []).map((opt: SelectOption) => ({
...opt,
selected: false,
}))
Expand All @@ -345,7 +345,7 @@ export const Select: FC<SelectProps> = React.forwardRef(
!prevDropdownVisible &&
filterable &&
!multiple &&
searchQuery.length !== 0 &&
searchQuery?.length !== 0 &&
getSelectedOptions().length === 0
) {
inputRef.current?.click();
Expand All @@ -360,7 +360,7 @@ export const Select: FC<SelectProps> = React.forwardRef(
}
if (value) {
setOptions(
options.map((opt: SelectOption) => ({
(options || []).map((opt: SelectOption) => ({
...opt,
hideOption: filterOption
? !filterOption(opt, value)
Expand All @@ -372,7 +372,7 @@ export const Select: FC<SelectProps> = React.forwardRef(
// When not in multiple mode and the value is empty
// deselect and execute onClear.
setOptions(
options.map((opt: SelectOption) => {
(options || []).map((opt: SelectOption) => {
const selected: boolean = multiple ? opt.selected : false;
return {
...opt,
Expand All @@ -391,7 +391,7 @@ export const Select: FC<SelectProps> = React.forwardRef(

const showDropdown = (show: boolean) => {
if (filterable || multiple) {
if (!multiple && options.length) {
if (!multiple && options?.length) {
return show;
}
return true;
Expand Down Expand Up @@ -455,7 +455,7 @@ export const Select: FC<SelectProps> = React.forwardRef(
]);

const showPills = (): boolean => {
const selected: SelectOption[] = options.filter(
const selected: SelectOption[] = (options || []).filter(
(opt: SelectOption) => opt.selected
);
const selectedCount: number = selected.length;
Expand Down Expand Up @@ -490,7 +490,7 @@ export const Select: FC<SelectProps> = React.forwardRef(
};

const getPills = (): JSX.Element => {
const selected: SelectOption[] = options.filter(
const selected: SelectOption[] = (options || []).filter(
(opt: SelectOption) => opt.selected
);

Expand Down Expand Up @@ -537,7 +537,7 @@ export const Select: FC<SelectProps> = React.forwardRef(
} else {
pills.push(pill());
}
if (pills.length === count && filled) {
if (pills?.length === count && filled) {
pills.push(
<Pill
classNames={countPillClassNames}
Expand All @@ -561,7 +561,7 @@ export const Select: FC<SelectProps> = React.forwardRef(
setSelectedOptionText(searchQuery);
return;
}
const selectedOption = options
const selectedOption = (options || [])
.filter((opt: SelectOption) => opt.selected)
.map((opt: SelectOption) => opt.text)
.join(', ')
Expand All @@ -574,7 +574,7 @@ export const Select: FC<SelectProps> = React.forwardRef(
}: {
options: SelectOption[];
}): JSX.Element => {
const filteredOptions = options.filter(
const filteredOptions = (options || []).filter(
(opt: SelectOption) => !opt.hideOption
);
const updatedItems: SelectOption[] = filteredOptions.map(
Expand Down Expand Up @@ -669,7 +669,7 @@ export const Select: FC<SelectProps> = React.forwardRef(
]);

const selectInputProps: TextInputProps = {
placeholder: showPills() ? '' : placeholder,
placeholder: showPills() && !!options ? '' : placeholder,
alignIcon: TextInputIconAlign.Right,
clearable: clearable,
clearButtonClassNames: clearButtonClassNames,
Expand Down Expand Up @@ -755,8 +755,8 @@ export const Select: FC<SelectProps> = React.forwardRef(
dropdownVisible &&
(showEmptyDropdown ||
isLoading ||
searchQuery.length > 0 ||
options.length > 0)
searchQuery?.length > 0 ||
options?.length > 0)
}
ref={dropdownRef}
>
Expand Down
Loading