Skip to content

Commit c5b903f

Browse files
committed
misc: remove extraneous dep list from Effect in custom hook useCallbackRef; remove extraneous useMemo wrapper for isMenuTopPosition calculation in custom hook useMenuPositioner
1 parent 36caf7d commit c5b903f

File tree

9 files changed

+50
-52
lines changed

9 files changed

+50
-52
lines changed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
"@babel/preset-env": "^7.20.2",
5454
"@babel/preset-react": "^7.18.6",
5555
"@babel/preset-typescript": "^7.18.6",
56-
"@rollup/plugin-babel": "^6.0.2",
56+
"@rollup/plugin-babel": "^6.0.3",
5757
"@rollup/plugin-replace": "^5.0.1",
5858
"@rollup/plugin-typescript": "^9.0.2",
5959
"@storybook/addon-storysource": "^6.5.13",

src/Select.tsx

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -273,8 +273,8 @@ const Select = forwardRef<SelectRef, SelectProps>((
273273
const debouncedInputValue = useDebounce<string>(inputValue, inputDelay);
274274

275275
// Custom ref objects
276-
const onSearchChangeRef = useCallbackRef(onSearchChange);
277-
const onOptionChangeRef = useCallbackRef(onOptionChange);
276+
const onSearchChangeFn = useCallbackRef(onSearchChange);
277+
const onOptionChangeFn = useCallbackRef(onOptionChange);
278278
const onSearchChangeIsFn = useLatestRef<boolean>(isFunction(onSearchChange));
279279
const onOptionChangeIsFn = useLatestRef<boolean>(isFunction(onOptionChange));
280280
const menuOpenRef = useLatestRef<boolean>(menuOpen);
@@ -421,9 +421,9 @@ const Select = forwardRef<SelectRef, SelectProps>((
421421
useEffect(() => {
422422
if (onSearchChangeIsFn.current && onChangeEvtValue.current) {
423423
onChangeEvtValue.current = false;
424-
onSearchChangeRef(debouncedInputValue);
424+
onSearchChangeFn(debouncedInputValue);
425425
}
426-
}, [onSearchChangeRef, debouncedInputValue]);
426+
}, [onSearchChangeFn, debouncedInputValue]);
427427

428428
/**
429429
* useUpdateEffect
@@ -437,9 +437,9 @@ const Select = forwardRef<SelectRef, SelectProps>((
437437
? selectedOption[0].data
438438
: null;
439439

440-
onOptionChangeRef(normalSelectedOpts);
440+
onOptionChangeFn(normalSelectedOpts);
441441
}
442-
}, [onOptionChangeRef, isMulti, selectedOption]);
442+
}, [onOptionChangeFn, isMulti, selectedOption]);
443443

444444
/**
445445
* useUpdateEffect
@@ -677,9 +677,10 @@ const Select = forwardRef<SelectRef, SelectProps>((
677677
}, []);
678678

679679
const handleOnCaretMouseDown = useCallback((e: MouseOrTouchEvent<HTMLElement>): void => {
680-
if (isDisabled || openMenuOnClick) return;
681-
handleOnMouseDown(e);
682-
menuOpenRef.current ? setMenuOpen(false) : openMenuAndFocusOption(OptionIndexEnum.FIRST);
680+
if (!isDisabled && !openMenuOnClick) {
681+
handleOnMouseDown(e);
682+
menuOpenRef.current ? setMenuOpen(false) : openMenuAndFocusOption(OptionIndexEnum.FIRST);
683+
}
683684
}, [isDisabled, openMenuOnClick, openMenuAndFocusOption]);
684685

685686
const flexValueWrapper = !!isMulti && hasSelectedOptions;

src/components/Menu/Option.tsx

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,11 @@ type OptionProps = Readonly<{
99
style: CSSProperties;
1010
}>;
1111

12-
// Extends react-window "areEqual" adding early bailout based on "memoOptions" flag
13-
const _areEqual = (prevProps: OptionProps, nextProps: OptionProps): boolean => {
12+
// extends react-window 'areEqual'
13+
const _areEqual = (
14+
prevProps: OptionProps,
15+
nextProps: OptionProps
16+
): boolean => {
1417
const { memoOptions } = nextProps.data;
1518
return memoOptions && areEqual(prevProps, nextProps);
1619
};
@@ -27,7 +30,7 @@ const Option = memo<OptionProps>(({
2730
}) => {
2831
const opt = menuOptions[index];
2932

30-
const _className = buildOptionClsName(
33+
const className = buildOptionClsName(
3134
opt.isDisabled,
3235
opt.isSelected,
3336
index === focusedOptionIndex
@@ -36,7 +39,7 @@ const Option = memo<OptionProps>(({
3639
return (
3740
<div
3841
style={style}
39-
className={_className}
42+
className={className}
4043
onClick={() => selectOption(opt)}
4144
>
4245
{renderOptionLabel(opt.data)}

src/components/Menu/index.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ const MenuWrapper = styled.div<MenuWrapperProps>`
5252
white-space: nowrap;
5353
text-overflow: ellipsis;
5454
-webkit-tap-highlight-color: transparent;
55-
will-change: top;
5655
padding: ${({ theme }) => theme.menu.option.padding};
5756
text-align: ${({ theme }) => theme.menu.option.textAlign};
5857

src/hooks/useCallbackRef.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@ import { useEffect, useRef, useCallback } from 'react';
55
* Hook that converts a callback to a ref to avoid triggering re-renders when
66
* passed as a prop or avoid re-executing effects when passed as a dependency
77
*
8-
* @param callback the callback to write to a ref object
8+
* @param callback the callback to write to ref object
99
*/
10-
const useCallbackRef = <T extends CallbackFn>(callback: T | undefined): T => {
10+
const useCallbackRef = <T extends CallbackFn>(callback?: T): T => {
1111
const callbackRef = useRef(callback);
1212

1313
useEffect(() => {
1414
callbackRef.current = callback;
15-
}, [callback]);
15+
});
1616

1717
return useCallback(
1818
((...args) => {

src/hooks/useMenuOptions.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ const useMenuOptions = (
3030
async: boolean = false,
3131
hideSelectedOptions?: boolean
3232
): MenuOption[] => {
33-
const getFilterOptionStringRef = useCallbackRef(getFilterOptionString || FUNCTIONS.optionFilter);
34-
const getIsOptionDisabledRef = useCallbackRef(getIsOptionDisabled || FUNCTIONS.isOptionDisabled);
33+
const getIsOptionDisabledFn = useCallbackRef(getIsOptionDisabled || FUNCTIONS.isOptionDisabled);
34+
const getFilterOptionStringFn = useCallbackRef(getFilterOptionString || FUNCTIONS.optionFilter);
3535
const hideSelectedOptionsOrDefault = isBoolean(hideSelectedOptions) ? hideSelectedOptions : isMulti;
36-
const searchValue = !async ? debouncedInputValue : ''; // Prevent recomputing/filtering on input mutations in async mode
36+
const searchValue = !async ? debouncedInputValue : ''; // prevent recomputing on input mutations in async mode
3737

3838
const menuOptions = useMemo<MenuOption[]>(() => {
3939
const selectedValues = selectedOption.map((x) => x.value);
@@ -42,15 +42,15 @@ const useMenuOptions = (
4242

4343
const isOptionFilterMatch = (option: MenuOption): boolean => {
4444
if (!matchVal) return true;
45-
const filterVal = getFilterOptionStringRef(option);
45+
const filterVal = getFilterOptionStringFn(option);
4646
const normalFilterVal = trimAndFormatFilterStr(filterVal, filterIgnoreCase, filterIgnoreAccents);
4747
return isFilterMatchAny ? normalFilterVal.includes(matchVal) : normalFilterVal.startsWith(matchVal);
4848
};
4949

5050
const parseMenuOption = (data: OptionData): MenuOption | undefined => {
5151
const value = getOptionValue(data);
5252
const label = getOptionLabel(data);
53-
const isDisabled = getIsOptionDisabledRef(data);
53+
const isDisabled = getIsOptionDisabledFn(data);
5454
const isSelected = selectedValues.includes(value);
5555
const menuOption: MenuOption = { data, value, label, isDisabled, isSelected };
5656
return (!isOptionFilterMatch(menuOption) || (hideSelectedOptionsOrDefault && isSelected))
@@ -72,8 +72,8 @@ const useMenuOptions = (
7272
filterMatchFrom,
7373
filterIgnoreCase,
7474
filterIgnoreAccents,
75-
getIsOptionDisabledRef,
76-
getFilterOptionStringRef,
75+
getIsOptionDisabledFn,
76+
getFilterOptionStringFn,
7777
hideSelectedOptionsOrDefault
7878
]);
7979

src/hooks/useMenuPositioner.ts

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import useLatestRef from './useLatestRef';
33
import useCallbackRef from './useCallbackRef';
44
import useUpdateEffect from './useUpdateEffect';
55
import { MenuPositionEnum } from '../constants';
6-
import { useMemo, useState, useRef, type RefObject } from 'react';
6+
import { useState, useRef, type RefObject } from 'react';
77
import { calculateMenuTop, menuFitsBelowControl, scrollMenuIntoViewOnOpen } from '../utils';
88

99
/**
@@ -27,21 +27,20 @@ const useMenuPositioner = (
2727
menuScrollDuration?: number,
2828
scrollMenuIntoView?: boolean
2929
): [string | undefined, number] => {
30-
const isMenuTopPosition = useMemo<boolean>(() => {
31-
return menuPosition === MenuPositionEnum.TOP ||
32-
(menuPosition === MenuPositionEnum.AUTO && !menuFitsBelowControl(menuRef.current));
33-
}, [menuRef, menuPosition]);
30+
const isMenuTopPosition =
31+
menuPosition === MenuPositionEnum.TOP ||
32+
(menuPosition === MenuPositionEnum.AUTO && !menuFitsBelowControl(menuRef.current));
3433

35-
const onMenuOpenRef = useCallbackRef(onMenuOpen);
36-
const onMenuCloseRef = useCallbackRef(onMenuClose);
34+
const onMenuOpenFn = useCallbackRef(onMenuOpen);
35+
const onMenuCloseFn = useCallbackRef(onMenuClose);
3736
const resetMenuHeightRef = useRef<boolean>(false);
3837
const [menuHeight, setMenuHeight] = useState<number>(menuHeightDefault);
3938
const shouldScrollRef = useLatestRef<boolean>(!isMenuTopPosition && !isMenuPortaled);
4039

4140
useUpdateEffect(() => {
4241
if (menuOpen) {
4342
const handleOnMenuOpen = (availableSpace?: number) => {
44-
onMenuOpenRef();
43+
onMenuOpenFn();
4544
if (availableSpace) {
4645
resetMenuHeightRef.current = true;
4746
setMenuHeight(availableSpace);
@@ -57,7 +56,7 @@ const useMenuPositioner = (
5756
)
5857
: handleOnMenuOpen();
5958
} else {
60-
onMenuCloseRef();
59+
onMenuCloseFn();
6160
if (resetMenuHeightRef.current) {
6261
resetMenuHeightRef.current = false;
6362
setMenuHeight(menuHeightDefault);
@@ -69,11 +68,11 @@ const useMenuPositioner = (
6968
menuHeightDefault,
7069
scrollMenuIntoView,
7170
menuScrollDuration,
72-
onMenuCloseRef,
73-
onMenuOpenRef,
71+
onMenuOpenFn,
72+
onMenuCloseFn
7473
]);
7574

76-
// Calculated menu height passed react-window; calculate MenuWrapper <div /> 'top' style prop if menu is positioned above control
75+
// Calculated menu height passed react-window; calculate MenuWrapper div 'top' style prop if menu is positioned above control
7776
const menuHeightCalc = Math.min(menuHeight, menuOptionsLength * menuItemSize);
7877
const menuStyleTop = isMenuTopPosition ? calculateMenuTop(menuHeightCalc, menuRef.current, controlRef.current) : undefined;
7978

src/utils/common.ts

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,7 @@ export const trimAndFormatFilterStr = (
3434
filterIgnoreAccents: boolean
3535
): string => {
3636
let trimVal = value.trim();
37-
if (filterIgnoreCase) {
38-
trimVal = trimVal.toLowerCase();
39-
}
37+
if (filterIgnoreCase) trimVal = trimVal.toLowerCase();
4038
return !filterIgnoreAccents ? trimVal : stripDiacritics(trimVal);
4139
};
4240

@@ -48,16 +46,16 @@ export const buildOptionClsName = (
4846
isSelected: boolean,
4947
isFocused: boolean
5048
): string => {
51-
let className = OPTION_CLS;
49+
let cx = OPTION_CLS;
5250

5351
if (isDisabled)
54-
className += ` ${OPTION_DISABLED_CLS}`;
52+
cx += ' ' + OPTION_DISABLED_CLS;
5553
if (isSelected)
56-
className += ` ${OPTION_SELECTED_CLS}`;
54+
cx += ' ' + OPTION_SELECTED_CLS;
5755
if (isFocused)
58-
className += ` ${OPTION_FOCUSED_CLS}`;
56+
cx += ' ' + OPTION_FOCUSED_CLS;
5957

60-
return className;
58+
return cx;
6159
};
6260

6361
/**
@@ -68,17 +66,17 @@ export const normalizeValue = (
6866
getOptionValue: OptionValueCallback,
6967
getOptionLabel: OptionLabelCallback
7068
): SelectedOption[] => {
71-
const initValues = Array.isArray(value)
69+
const initVals = Array.isArray(value)
7270
? value
7371
: isPlainObject(value)
7472
? [value]
7573
: EMPTY_ARRAY;
7674

77-
if (!isArrayWithLength(initValues)) {
78-
return initValues;
75+
if (!isArrayWithLength(initVals)) {
76+
return initVals;
7977
}
8078

81-
return initValues.map((data: unknown) => ({
79+
return initVals.map((data) => ({
8280
data,
8381
value: getOptionValue(data),
8482
label: getOptionLabel(data)

src/utils/device.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@ let _isTouchDevice: boolean | undefined;
77
* Global, lazy evaluation.
88
*/
99
export const isTouchDevice = (): boolean => {
10-
if (isBoolean(_isTouchDevice)) {
11-
return _isTouchDevice;
12-
}
10+
if (isBoolean(_isTouchDevice)) return _isTouchDevice;
1311

1412
return (_isTouchDevice = (() => {
1513
try {

0 commit comments

Comments
 (0)