Here is a code of a search filter component where user can find products by typing letters inside the input field. I wonder which improvements can i add. I was thinking of removing some logic into custom hook, but don’t consider it necessary since I use it only once.
p.s. I know that it would be better to use sth like Stack Exchange for such kind of question, but I wanna get the answer.
Component:
export const SearchItems = ({
openBackdrop,
setOpenBackDrop,
}: SearchItemsProps) => {
const theme = useTheme();
const [searchQ, setSearchQ] = useState<string>("");
const [isScrollDisabled, setIsScrollDisabled] = useState<boolean>(false);
const [products, setProducts] = useState<Product[]>([]);
const [productsByQ, setProductsByQ] = useState<Product[]>([]);
useEffect(() => {
const getProducts = async () => {
try {
const productsRef = await getDocs(collection(db, "products"));
const products = productsRef.docs.map((doc) => ({
...doc.data(),
}));
const flattenData: Product[] = products.flatMap(
(product) => product.data
);
setProducts(flattenData);
} catch (error: any) {
console.log(error.message);
}
};
getProducts();
}, []);
const handleFilterProductsByQ = useMemo(
() =>
debounce((searchQ: string) => {
const filteredData = products.filter((product) =>
product.name.toLowerCase().includes(searchQ.toLowerCase())
);
setProductsByQ(filteredData);
}, 300),
[products]
);
useEffect(() => {
handleFilterProductsByQ(searchQ);
}, [searchQ]);
useEffect(() => {
isScrollDisabled
? (document.body.style.overflow = "hidden")
: (document.body.style.overflow = "auto");
return () => {
document.body.style.overflow = "auto";
};
}, [isScrollDisabled]);
const handleInputFocus = () => {
window.scrollTo({ top: 0, behavior: "auto" });
setIsScrollDisabled(true);
setOpenBackDrop(true);
};
const handleOutsideClick = () => {
setOpenBackDrop(false);
setIsScrollDisabled(false);
setSearchQ("");
};
return (
<OutsideClickHandler onOutsideClick={handleOutsideClick}>
<Box position="relative">
<OutlinedInput
type="text"
placeholder="Search Item…"
value={searchQ}
onChange={(e) => setSearchQ(e.target.value)}
onFocus={handleInputFocus}
startAdornment={
<InputAdornment
position="start"
sx={{
color: theme.palette.primary.main,
fontSize: "1.2rem",
}}
>
<FiSearch />
</InputAdornment>
}
endAdornment={
<InputAdornment position="end">
<IconButton
edge="end"
onClick={() => setSearchQ("")}
sx={{
color: theme.palette.secondary.light,
cursor: "pointer",
fontSize: "0.1rem",
}}
>
{searchQ !== "" && <CloseIcon />}
</IconButton>
</InputAdornment>
}
sx={{
borderRadius: "0.8rem",
border: "none",
backgroundColor:
openBackdrop === true ? "#f9fafb" : theme.palette.primary.light,
zIndex: openBackdrop === true ? 9999 : "none",
"& .MuiOutlinedInput-notchedOutline": {
border: "none",
},
}}
/>
{searchQ.length !== 0 && (
<SearchResult searchQ={searchQ} productsByQ={productsByQ} />
)}
</Box>
</OutsideClickHandler>
);
};
2
Answers
You don’t need this
const [productsByQ, setProductsByQ] = useState<Product[]>([]);
and the whole logic around it. So you already have a state that holds theproducts
and a state for thesearch term
.Every time your search term changes, it triggers a component re-render thus you can directly use your search term and products like this:
Then you can move your debounce logic for the search term thus you won’t be filtering by every key stroke
I’m not a TypeScript developer, so the given code is simply using JavaScript or the types you already had present. But if I’m not mistaken most types in these scenario’s can be inferred by TypeScript. If not you’ll need to add them yourself. With that out of the way let’s get to the first point.
debounce
makes the component way more complex than it needs to be. If possible I would look intouseDeferredValue
.Let me give an example using your given code:
This would remove the need for the following code:
The above already shrinks the component quite a bit. If you need more speed during the first render, or renders where the search query is empty you could short circuit the
useMemo
and skip the.filter()
if the query is empty. This does grow the complexity, but is still simpler than the logic you had before.I would also suggest to use
memo
(if you haven’t already) to memoizeSearchResult
. This lets you skip re-rendering theSearchResult
component when its props are unchanged. This can happen quite a lot when used in combination withuseDeferredValue
.Another thing to note is that you currently use the conditional (ternary) operator
… ? … : …
as a substitution for a normalif
/else
statement.I would personally use the fact that the conditional (ternary) operator is an expression (it returns a value) and is not just a statement. The above could be changed into:
Since the clean-up function only contains a single statement that could also function as an expression, you can remove the block delimiters of the clean-up function and keep the expression on the same line. This will change the return type of the clean-up function, but since the return value is not used by React that shouldn’t matter.
Finally, most of the time
someVariable === true
(orsomeVariable === false
) is an anti-pattern when used inside anif
-statement or conditional (ternary) operator. This can often be replaced by justsomeVariable
(or!soveVariable
) to check for a truthy/falsy value.In your scenario: