-
-
Notifications
You must be signed in to change notification settings - Fork 874
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.
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
perf: optimize components with memoization and lazy loading - Add use… #3902
base: master
Are you sure you want to change the base?
Conversation
…Callback/useMemo to filter components, implement lazy loading for images, optimize ToolsDashboard
WalkthroughThis pull request makes several formatting and refactoring updates across various components. In the Avatar component, a Prettier disable comment is removed and JSX attributes are consolidated. The Figure and SponsorImage components now include a lazy-loading attribute on their Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant TD as ToolsDashboard
participant F as Filter Functions
U->>TD: Click outside filter/modal area
TD->>TD: Invoke handleClickOutsideFilter/handleClickOutsideCategory (useCallback)
TD->>F: Apply filterToolsByCategory & checkToolFilters
F-->>TD: Return filtered list
TD-->>U: Render updated tools list
Suggested labels
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
components/tools/ToolsDashboard.tsx (1)
117-136
:⚠️ Potential issueFix
let
toconst
and add formatting improvementsUsing
useMemo
for the tools list calculation is an excellent optimization. The dependencies array correctly includes all the required functions and state variables.However, there's an issue with the variable declaration and formatting:
Fix the variable declaration and formatting:
const toolsList = useMemo(() => { - let tempToolsList = filterToolsByCategory({}, categories); + const tempToolsList = filterToolsByCategory({}, categories); + setCheckToolsList(false); - + Object.keys(tempToolsList).forEach((category) => { tempToolsList[category].toolsList = tempToolsList[category].toolsList.filter(checkToolFilters); if (tempToolsList[category].toolsList.length) { setCheckToolsList(true); } }); Object.keys(tempToolsList).map((category) => { tempToolsList[category].elementRef = createRef(); return tempToolsList; }); return tempToolsList; }, [categories, checkToolFilters, filterToolsByCategory]);🧰 Tools
🪛 ESLint
[error] 118-118: Expected blank line after variable declarations.
(newline-after-var)
[error] 118-118: 'tempToolsList' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 119-119: Expected blank line before this statement.
(padding-line-between-statements)
🪛 GitHub Actions: PR testing - if Node project
[error] 118-118: Error: 'tempToolsList' is never reassigned. Use 'const' instead. prefer-const
🧹 Nitpick comments (4)
components/tools/FiltersDropdown.tsx (2)
31-40
: Excellent performance optimization with useCallback.Wrapping
handleClickOption
withuseCallback
prevents unnecessary function recreations on re-renders, which is particularly valuable for callback functions passed to child components. This optimization will prevent unnecessary re-renders of child components that receive this function as a prop.Fix the minor formatting issues:
- Add a blank line after the variable declaration on line 34
- Remove trailing spaces on lines 34 and 35
Apply these fixes:
const handleClickOption = useCallback( (option: string) => { const isChecked = checkedOptions.includes(option); - const updatedOptions = isChecked - ? checkedOptions.filter((item) => item !== option) + const updatedOptions = isChecked + ? checkedOptions.filter((item) => item !== option) : [...checkedOptions, option]; + setCheckedOptions(updatedOptions); }, [checkedOptions, setCheckedOptions] );🧰 Tools
🪛 ESLint
[error] 34-36: Expected blank line after variable declarations.
(newline-after-var)
[error] 34-34: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 34-36: Replace
⏎········?·checkedOptions.filter((item)·=>·item·!==·option)·⏎·······
with?·checkedOptions.filter((item)·=>·item·!==·option)
(prettier/prettier)
[error] 35-35: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 37-37: Expected blank line before this statement.
(padding-line-between-statements)
🪛 GitHub Actions: PR testing - if Node project
[error] 34-34: Error: Expected blank line after variable declarations. newline-after-var
42-47
: Great optimization with useMemo for list rendering.Using
useMemo
for thecheckboxItems
is an excellent optimization that prevents recreating the array of Checkbox components on every render. This will only recalculate when dependencies change, improving performance especially with larger lists.Fix the minor formatting issues:
- Add a blank line after the variable declaration on line 44
- Add a blank line before the return statement on line 45
Apply these fixes:
const checkboxItems = useMemo(() => { return dataList.map((data, index) => { const checked = checkedOptions.includes(data.name); + return <Checkbox key={index} name={data.name} checked={checked} handleClickOption={handleClickOption} />; }); }, [dataList, checkedOptions, handleClickOption]);
🧰 Tools
🪛 ESLint
[error] 44-44: Expected blank line after variable declarations.
(newline-after-var)
[error] 45-45: Expected blank line before this statement.
(padding-line-between-statements)
components/tools/ToolsDashboard.tsx (2)
44-56
: Good update to useEffect dependenciesThe
useEffect
hooks now correctly use the memoized callback functions in their dependency arrays. This ensures the effects only run when the respective callback function changes.However, the code needs blank lines before the return statements to match the project's code style:
useEffect(() => { document.addEventListener('mousedown', handleClickOutsideFilter); + return () => { document.removeEventListener('mousedown', handleClickOutsideFilter); }; }, [handleClickOutsideFilter]); useEffect(() => { document.addEventListener('mousedown', handleClickOutsideCategory); + return () => { document.removeEventListener('mousedown', handleClickOutsideCategory); }; }, [handleClickOutsideCategory]);🧰 Tools
🪛 ESLint
[error] 46-48: Expected blank line before this statement.
(padding-line-between-statements)
[error] 53-55: Expected blank line before this statement.
(padding-line-between-statements)
58-71
: Good extraction of filtering logic with useCallbackExtracting the category filtering logic into a separate
useCallback
function improves code organization and performance. The empty dependency array is correct since this function doesn't depend on any component state or props.However, there are formatting issues that need to be fixed:
const filterToolsByCategory = useCallback((tempToolsList: ToolsListData, selectedCategories: string[]) => { if (selectedCategories.length > 0) { const filteredTools: ToolsListData = {}; + for (const category of selectedCategories) { Object.keys(ToolsData).forEach((key) => { if (key === category) { filteredTools[key] = JSON.parse(JSON.stringify(ToolsData[key])); } }); } + return filteredTools; } + return JSON.parse(JSON.stringify(ToolsData)); }, []);🧰 Tools
🪛 ESLint
[error] 60-60: Expected blank line after variable declarations.
(newline-after-var)
[error] 61-67: Expected blank line before this statement.
(padding-line-between-statements)
[error] 68-68: Expected blank line before this statement.
(padding-line-between-statements)
[error] 70-70: Expected blank line before this statement.
(padding-line-between-statements)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
components/Avatar.tsx
(1 hunks)components/Figure.tsx
(1 hunks)components/sponsors/SponsorImage.tsx
(1 hunks)components/tools/FiltersDisplay.tsx
(2 hunks)components/tools/FiltersDropdown.tsx
(2 hunks)components/tools/ToolsDashboard.tsx
(3 hunks)
🧰 Additional context used
🪛 ESLint
components/Figure.tsx
[error] 41-41: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 41-47: Replace ⏎··········className={
${imageClass}}·⏎··········src={src}·⏎··········alt={alt}·⏎··········data-testid='Figure-img'⏎··········loading="lazy"⏎·······
with className={
${imageClass}}·src={src}·alt={alt}·data-testid='Figure-img'·loading='lazy'
(prettier/prettier)
[error] 42-42: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 43-43: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 44-44: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 46-46: Unexpected usage of doublequote.
(jsx-quotes)
components/sponsors/SponsorImage.tsx
[error] 17-17: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 17-17: Delete ·
(prettier/prettier)
[error] 21-21: Replace "lazy"
with 'lazy'
(prettier/prettier)
[error] 21-21: Unexpected usage of doublequote.
(jsx-quotes)
components/Avatar.tsx
[error] 27-27: Replace "lazy"
with 'lazy'
(prettier/prettier)
[error] 27-27: Unexpected usage of doublequote.
(jsx-quotes)
components/tools/ToolsDashboard.tsx
[error] 32-32: Insert ⏎····
(prettier/prettier)
[error] 33-33: Insert ··
(prettier/prettier)
[error] 34-34: Insert ··
(prettier/prettier)
[error] 35-35: Insert ··
(prettier/prettier)
[error] 36-36: Replace ··},·[openFilter]
with ····},⏎····[openFilter]⏎··
(prettier/prettier)
[error] 38-38: Insert ⏎····
(prettier/prettier)
[error] 39-39: Insert ··
(prettier/prettier)
[error] 40-40: Insert ··
(prettier/prettier)
[error] 41-41: Insert ··
(prettier/prettier)
[error] 42-42: Replace ··},·[openCategory]
with ····},⏎····[openCategory]⏎··
(prettier/prettier)
[error] 46-48: Expected blank line before this statement.
(padding-line-between-statements)
[error] 53-55: Expected blank line before this statement.
(padding-line-between-statements)
[error] 60-60: Expected blank line after variable declarations.
(newline-after-var)
[error] 61-67: Expected blank line before this statement.
(padding-line-between-statements)
[error] 68-68: Expected blank line before this statement.
(padding-line-between-statements)
[error] 70-70: Expected blank line before this statement.
(padding-line-between-statements)
[error] 73-73: Insert ⏎····
(prettier/prettier)
[error] 74-74: Replace ····
with ······
(prettier/prettier)
[error] 75-75: Insert ··
(prettier/prettier)
[error] 76-76: Insert ··
(prettier/prettier)
[error] 77-77: Insert ··
(prettier/prettier)
[error] 78-78: Insert ··
(prettier/prettier)
[error] 80-80: Insert ··
(prettier/prettier)
[error] 81-81: Insert ··
(prettier/prettier)
[error] 82-82: Insert ··
(prettier/prettier)
[error] 83-83: Replace ········
with ··········
(prettier/prettier)
[error] 84-84: Insert ··
(prettier/prettier)
[error] 85-85: Insert ··
(prettier/prettier)
[error] 86-86: Insert ··
(prettier/prettier)
[error] 87-87: Replace ····
with ······
(prettier/prettier)
[error] 89-89: Insert ··
(prettier/prettier)
[error] 90-90: Replace ······
with ········
(prettier/prettier)
[error] 91-91: Insert ··
(prettier/prettier)
[error] 92-92: Replace ········
with ··········
(prettier/prettier)
[error] 93-93: Insert ··
(prettier/prettier)
[error] 94-94: Insert ··
(prettier/prettier)
[error] 95-95: Insert ··
(prettier/prettier)
[error] 96-96: Replace ····
with ······
(prettier/prettier)
[error] 98-98: Insert ··
(prettier/prettier)
[error] 99-99: Insert ··
(prettier/prettier)
[error] 100-100: Insert ··
(prettier/prettier)
[error] 102-102: Insert ··
(prettier/prettier)
[error] 103-103: Insert ··
(prettier/prettier)
[error] 104-104: Insert ··
(prettier/prettier)
[error] 106-106: Insert ··
(prettier/prettier)
[error] 107-107: Insert ··
(prettier/prettier)
[error] 108-108: Insert ··
(prettier/prettier)
[error] 109-109: Insert ··
(prettier/prettier)
[error] 110-110: Insert ··
(prettier/prettier)
[error] 111-111: Insert ··
(prettier/prettier)
[error] 112-112: Replace ····
with ······
(prettier/prettier)
[error] 114-114: Insert ··
(prettier/prettier)
[error] 115-115: Replace },·[languages,·technologies,·searchName,·isAsyncAPIOwner,·isPaid]
with ··},⏎····[languages,·technologies,·searchName,·isAsyncAPIOwner,·isPaid]⏎··
(prettier/prettier)
[error] 118-118: Expected blank line after variable declarations.
(newline-after-var)
[error] 118-118: 'tempToolsList' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 119-119: Expected blank line before this statement.
(padding-line-between-statements)
components/tools/FiltersDropdown.tsx
[error] 34-36: Expected blank line after variable declarations.
(newline-after-var)
[error] 34-34: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 34-36: Replace ⏎········?·checkedOptions.filter((item)·=>·item·!==·option)·⏎·······
with ?·checkedOptions.filter((item)·=>·item·!==·option)
(prettier/prettier)
[error] 35-35: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 37-37: Expected blank line before this statement.
(padding-line-between-statements)
[error] 44-44: Expected blank line after variable declarations.
(newline-after-var)
[error] 45-45: Expected blank line before this statement.
(padding-line-between-statements)
🪛 GitHub Actions: PR testing - if Node project
components/Figure.tsx
[error] 1-1: Error: Run autofix to sort these imports! simple-import-sort/imports
[error] 41-41: Error: Trailing spaces not allowed. no-trailing-spaces
[error] 44-44: Error: Unexpected usage of doublequote. jsx-quotes
components/sponsors/SponsorImage.tsx
[error] 17-17: Error: Trailing spaces not allowed. no-trailing-spaces
[error] 21-21: Error: Replace '"lazy"' with ''lazy'' prettier/prettier
components/Avatar.tsx
[error] 4-4: Error: Expected line before comment. lines-around-comment
[error] 27-27: Error: Replace '"lazy"' with ''lazy'' prettier/prettier
[error] 27-27: Error: Unexpected usage of doublequote. jsx-quotes
components/tools/ToolsDashboard.tsx
[error] 32-32: Error: Insert '⏎····' prettier/prettier
[error] 34-34: Error: Expected blank line after variable declarations. newline-after-var
[error] 118-118: Error: 'tempToolsList' is never reassigned. Use 'const' instead. prefer-const
components/tools/FiltersDropdown.tsx
[error] 34-34: Error: Expected blank line after variable declarations. newline-after-var
🪛 Biome (1.9.4)
components/tools/ToolsDashboard.tsx
[error] 83-83: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 92-92: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (8)
components/sponsors/SponsorImage.tsx (1)
12-12
: Great documentation addition.The added comment provides valuable context about the component's design considerations.
components/tools/FiltersDropdown.tsx (2)
1-1
: Good addition of React hooks for optimization.Adding
useCallback
anduseMemo
to the imports prepares for the performance optimizations implemented throughout the file.
54-54
: Good implementation of the memoized component list.Using the memoized
checkboxItems
variable here instead of inline mapping improves readability and leverages the performance optimization from useMemo.components/tools/FiltersDisplay.tsx (4)
1-1
: Great addition of React hooks!Adding the
useCallback
anduseMemo
imports is appropriate for the performance optimizations being implemented.
16-27
: Good optimization with useCallbackWrapping the
handleClickOption
function withuseCallback
is a great performance improvement. This prevents the function from being recreated on each render, which is especially important for event handlers.The dependency array correctly includes
checkedOptions
andsetCheckedOptions
- the only external variables used within the function.
29-47
: Effective use of useMemo for complex UI elementsMemoizing the
displayItems
JSX structure is an excellent optimization. It prevents unnecessary recreation of these UI elements during re-renders, which can significantly improve performance for lists.The dependency array correctly includes both
checkedOptions
andhandleClickOption
.
49-51
: Clean JSX structure with memoized contentThe component now has a cleaner structure by using the memoized
displayItems
variable and including a proper null check before attempting to render. This improves both performance and readability.Also applies to: 54-56
components/tools/ToolsDashboard.tsx (1)
2-2
: Appropriate import additionsAdding
useCallback
to the imports is necessary for the optimizations being implemented throughout the component.
<img | ||
src={src} | ||
alt={alt} | ||
className={twMerge('max-h-9 sm:max-h-12 w-auto object-contain', className)} | ||
loading="lazy" | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent performance optimization with lazy loading, but fix formatting issues.
Adding loading="lazy"
to the image is a valuable performance optimization that will defer loading off-screen images until they're needed.
However, there are formatting issues that need to be addressed:
- Remove trailing whitespace on line 17
- Use single quotes instead of double quotes for the loading attribute
Apply these fixes:
- <img
+ <img
src={src}
alt={alt}
className={twMerge('max-h-9 sm:max-h-12 w-auto object-contain', className)}
- loading="lazy"
+ loading='lazy'
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<img | |
src={src} | |
alt={alt} | |
className={twMerge('max-h-9 sm:max-h-12 w-auto object-contain', className)} | |
loading="lazy" | |
/> | |
<img | |
src={src} | |
alt={alt} | |
className={twMerge('max-h-9 sm:max-h-12 w-auto object-contain', className)} | |
loading='lazy' | |
/> |
🧰 Tools
🪛 ESLint
[error] 17-17: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 17-17: Delete ·
(prettier/prettier)
[error] 21-21: Replace "lazy"
with 'lazy'
(prettier/prettier)
[error] 21-21: Unexpected usage of doublequote.
(jsx-quotes)
🪛 GitHub Actions: PR testing - if Node project
[error] 17-17: Error: Trailing spaces not allowed. no-trailing-spaces
[error] 21-21: Error: Replace '"lazy"' with ''lazy'' prettier/prettier
<img | ||
className={`${imageClass}`} | ||
src={src} | ||
alt={alt} | ||
data-testid='Figure-img' | ||
loading="lazy" | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good performance optimization with lazy loading, but fix formatting issues.
Adding loading="lazy"
to the image is a valuable performance optimization that will defer loading off-screen images until they're needed.
However, there are formatting issues that need to be addressed:
- Remove trailing whitespace on lines 41-44
- Use single quotes instead of double quotes for the loading attribute
- Sort the imports at the top of the file
Apply these fixes:
- <img
- className={`${imageClass}`}
- src={src}
- alt={alt}
+ <img
+ className={`${imageClass}`}
+ src={src}
+ alt={alt}
data-testid='Figure-img'
- loading="lazy"
+ loading='lazy'
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<img | |
className={`${imageClass}`} | |
src={src} | |
alt={alt} | |
data-testid='Figure-img' | |
loading="lazy" | |
/> | |
<img | |
className={`${imageClass}`} | |
src={src} | |
alt={alt} | |
data-testid='Figure-img' | |
loading='lazy' | |
/> |
🧰 Tools
🪛 ESLint
[error] 41-41: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 41-47: Replace ⏎··········className={
${imageClass}}·⏎··········src={src}·⏎··········alt={alt}·⏎··········data-testid='Figure-img'⏎··········loading="lazy"⏎·······
with className={
${imageClass}}·src={src}·alt={alt}·data-testid='Figure-img'·loading='lazy'
(prettier/prettier)
[error] 42-42: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 43-43: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 44-44: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 46-46: Unexpected usage of doublequote.
(jsx-quotes)
🪛 GitHub Actions: PR testing - if Node project
[error] 41-41: Error: Trailing spaces not allowed. no-trailing-spaces
[error] 44-44: Error: Unexpected usage of doublequote. jsx-quotes
loading="lazy" | ||
data-testid='Avatars-img' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good performance optimization with lazy loading, but fix formatting issues.
Adding loading="lazy"
to the image is a valuable performance optimization that will defer loading off-screen images until they're needed.
However, there are formatting issues that need to be addressed:
- Use single quotes instead of double quotes for the loading attribute
- Add a blank line before the interface comment on line 4 (as reported by the linter)
Apply this fix:
- loading="lazy"
+ loading='lazy'
data-testid='Avatars-img'
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
loading="lazy" | |
data-testid='Avatars-img' | |
loading='lazy' | |
data-testid='Avatars-img' |
🧰 Tools
🪛 ESLint
[error] 27-27: Replace "lazy"
with 'lazy'
(prettier/prettier)
[error] 27-27: Unexpected usage of doublequote.
(jsx-quotes)
🪛 GitHub Actions: PR testing - if Node project
[error] 27-27: Error: Replace '"lazy"' with ''lazy'' prettier/prettier
[error] 27-27: Error: Unexpected usage of doublequote. jsx-quotes
const handleClickOutsideFilter = useCallback((event: MouseEvent) => { | ||
if (openFilter && filterRef.current && !filterRef.current.contains(event.target as Node)) { | ||
setOpenFilter(false); | ||
} | ||
}, [openFilter]); | ||
|
||
document.addEventListener('mousedown', checkIfClickOutside); | ||
const handleClickOutsideCategory = useCallback((event: MouseEvent) => { | ||
if (openCategory && categoryRef.current && !categoryRef.current.contains(event.target as Node)) { | ||
setopenCategory(false); | ||
} | ||
}, [openCategory]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix formatting and properly implement useCallback
The click handlers for detecting outside clicks have been correctly extracted into useCallback
functions, which is good for performance. However, there are formatting issues that need to be fixed.
Fix the formatting to match project standards and pass the CI pipeline:
- const handleClickOutsideFilter = useCallback((event: MouseEvent) => {
- if (openFilter && filterRef.current && !filterRef.current.contains(event.target as Node)) {
- setOpenFilter(false);
- }
- }, [openFilter]);
+ const handleClickOutsideFilter = useCallback(
+ (event: MouseEvent) => {
+ if (openFilter && filterRef.current && !filterRef.current.contains(event.target as Node)) {
+ setOpenFilter(false);
+ }
+ },
+ [openFilter]
+ );
- const handleClickOutsideCategory = useCallback((event: MouseEvent) => {
- if (openCategory && categoryRef.current && !categoryRef.current.contains(event.target as Node)) {
- setopenCategory(false);
- }
- }, [openCategory]);
+ const handleClickOutsideCategory = useCallback(
+ (event: MouseEvent) => {
+ if (openCategory && categoryRef.current && !categoryRef.current.contains(event.target as Node)) {
+ setopenCategory(false);
+ }
+ },
+ [openCategory]
+ );
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const handleClickOutsideFilter = useCallback((event: MouseEvent) => { | |
if (openFilter && filterRef.current && !filterRef.current.contains(event.target as Node)) { | |
setOpenFilter(false); | |
} | |
}, [openFilter]); | |
document.addEventListener('mousedown', checkIfClickOutside); | |
const handleClickOutsideCategory = useCallback((event: MouseEvent) => { | |
if (openCategory && categoryRef.current && !categoryRef.current.contains(event.target as Node)) { | |
setopenCategory(false); | |
} | |
}, [openCategory]); | |
const handleClickOutsideFilter = useCallback( | |
(event: MouseEvent) => { | |
if (openFilter && filterRef.current && !filterRef.current.contains(event.target as Node)) { | |
setOpenFilter(false); | |
} | |
}, | |
[openFilter] | |
); | |
const handleClickOutsideCategory = useCallback( | |
(event: MouseEvent) => { | |
if (openCategory && categoryRef.current && !categoryRef.current.contains(event.target as Node)) { | |
setopenCategory(false); | |
} | |
}, | |
[openCategory] | |
); |
🧰 Tools
🪛 ESLint
[error] 32-32: Insert ⏎····
(prettier/prettier)
[error] 33-33: Insert ··
(prettier/prettier)
[error] 34-34: Insert ··
(prettier/prettier)
[error] 35-35: Insert ··
(prettier/prettier)
[error] 36-36: Replace ··},·[openFilter]
with ····},⏎····[openFilter]⏎··
(prettier/prettier)
[error] 38-38: Insert ⏎····
(prettier/prettier)
[error] 39-39: Insert ··
(prettier/prettier)
[error] 40-40: Insert ··
(prettier/prettier)
[error] 41-41: Insert ··
(prettier/prettier)
[error] 42-42: Replace ··},·[openCategory]
with ····},⏎····[openCategory]⏎··
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
[error] 32-32: Error: Insert '⏎····' prettier/prettier
[error] 34-34: Error: Expected blank line after variable declarations. newline-after-var
const checkToolFilters = useCallback((tool: any) => { | ||
let isLanguageTool = true; | ||
let isTechnologyTool = true; | ||
let isSearchTool = true; | ||
let isAsyncAPITool = true; | ||
let isPaidTool = true; | ||
|
||
if (languages.length) { | ||
isLanguageTool = false; | ||
for (const language of languages) { | ||
if (tool?.filters?.language && tool.filters.language.find((item: any) => item.name === language)) { | ||
isLanguageTool = true; | ||
} | ||
} | ||
} | ||
|
||
if (technologies.length) { | ||
isTechnologyTool = false; | ||
for (const technology of technologies) { | ||
if (tool?.filters?.technology && tool.filters.technology.find((item) => item.name === technology)) { | ||
isTechnologyTool = true; | ||
} | ||
} | ||
if (technologies.length) { | ||
isTechnologyTool = false; | ||
for (const technology of technologies) { | ||
if (tool?.filters?.technology && tool.filters.technology.find((item: any) => item.name === technology)) { | ||
isTechnologyTool = true; | ||
} | ||
} | ||
} | ||
|
||
if (searchName) { | ||
isSearchTool = tool.title.toLowerCase().includes(searchName.toLowerCase()); | ||
} | ||
if (searchName) { | ||
isSearchTool = tool.title.toLowerCase().includes(searchName.toLowerCase()); | ||
} | ||
|
||
if (isAsyncAPIOwner) { | ||
isAsyncAPITool = tool.filters.isAsyncAPIOwner === isAsyncAPIOwner; | ||
} | ||
if (isAsyncAPIOwner) { | ||
isAsyncAPITool = tool.filters.isAsyncAPIOwner === isAsyncAPIOwner; | ||
} | ||
|
||
if (isPaid !== 'all') { | ||
if (isPaid === 'free') { | ||
isPaidTool = tool.filters.hasCommercial === false; | ||
} else { | ||
isPaidTool = tool.filters.hasCommercial === true; | ||
} | ||
} | ||
if (isPaid !== 'all') { | ||
if (isPaid === 'free') { | ||
isPaidTool = tool.filters.hasCommercial === false; | ||
} else { | ||
isPaidTool = tool.filters.hasCommercial === true; | ||
} | ||
} | ||
|
||
return isLanguageTool && isTechnologyTool && isSearchTool && isAsyncAPITool && isPaidTool; | ||
}, [languages, technologies, searchName, isAsyncAPIOwner, isPaid]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix formatting and use optional chaining for the filter function
The checkToolFilters
function is a good extraction of complex filtering logic into a reusable, memoized function. This improves organization and prevents unnecessary recalculations.
However, there are several formatting issues and opportunities for code improvement:
Fix the formatting and code structure:
- const checkToolFilters = useCallback((tool: any) => {
- let isLanguageTool = true;
- let isTechnologyTool = true;
- let isSearchTool = true;
- let isAsyncAPITool = true;
- let isPaidTool = true;
-
- if (languages.length) {
- isLanguageTool = false;
- for (const language of languages) {
- if (tool?.filters?.language && tool.filters.language.find((item: any) => item.name === language)) {
- isLanguageTool = true;
+ const checkToolFilters = useCallback(
+ (tool: any) => {
+ let isLanguageTool = true;
+ let isTechnologyTool = true;
+ let isSearchTool = true;
+ let isAsyncAPITool = true;
+ let isPaidTool = true;
+
+ if (languages.length) {
+ isLanguageTool = false;
+ for (const language of languages) {
+ if (tool?.filters?.language?.find((item: any) => item.name === language)) {
+ isLanguageTool = true;
+ }
}
- }
- }
+ }
- if (technologies.length) {
- isTechnologyTool = false;
- for (const technology of technologies) {
- if (tool?.filters?.technology && tool.filters.technology.find((item: any) => item.name === technology)) {
- isTechnologyTool = true;
+ if (technologies.length) {
+ isTechnologyTool = false;
+ for (const technology of technologies) {
+ if (tool?.filters?.technology?.find((item: any) => item.name === technology)) {
+ isTechnologyTool = true;
+ }
}
- }
- }
+ }
- if (searchName) {
- isSearchTool = tool.title.toLowerCase().includes(searchName.toLowerCase());
- }
+ if (searchName) {
+ isSearchTool = tool.title.toLowerCase().includes(searchName.toLowerCase());
+ }
- if (isAsyncAPIOwner) {
- isAsyncAPITool = tool.filters.isAsyncAPIOwner === isAsyncAPIOwner;
- }
+ if (isAsyncAPIOwner) {
+ isAsyncAPITool = tool.filters.isAsyncAPIOwner === isAsyncAPIOwner;
+ }
- if (isPaid !== 'all') {
- if (isPaid === 'free') {
- isPaidTool = tool.filters.hasCommercial === false;
- } else {
- isPaidTool = tool.filters.hasCommercial === true;
- }
- }
+ if (isPaid !== 'all') {
+ if (isPaid === 'free') {
+ isPaidTool = tool.filters.hasCommercial === false;
+ } else {
+ isPaidTool = tool.filters.hasCommercial === true;
+ }
+ }
- return isLanguageTool && isTechnologyTool && isSearchTool && isAsyncAPITool && isPaidTool;
- }, [languages, technologies, searchName, isAsyncAPIOwner, isPaid]);
+ return isLanguageTool && isTechnologyTool && isSearchTool && isAsyncAPITool && isPaidTool;
+ },
+ [languages, technologies, searchName, isAsyncAPIOwner, isPaid]
+ );
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const checkToolFilters = useCallback((tool: any) => { | |
let isLanguageTool = true; | |
let isTechnologyTool = true; | |
let isSearchTool = true; | |
let isAsyncAPITool = true; | |
let isPaidTool = true; | |
if (languages.length) { | |
isLanguageTool = false; | |
for (const language of languages) { | |
if (tool?.filters?.language && tool.filters.language.find((item: any) => item.name === language)) { | |
isLanguageTool = true; | |
} | |
} | |
} | |
if (technologies.length) { | |
isTechnologyTool = false; | |
for (const technology of technologies) { | |
if (tool?.filters?.technology && tool.filters.technology.find((item) => item.name === technology)) { | |
isTechnologyTool = true; | |
} | |
} | |
if (technologies.length) { | |
isTechnologyTool = false; | |
for (const technology of technologies) { | |
if (tool?.filters?.technology && tool.filters.technology.find((item: any) => item.name === technology)) { | |
isTechnologyTool = true; | |
} | |
} | |
} | |
if (searchName) { | |
isSearchTool = tool.title.toLowerCase().includes(searchName.toLowerCase()); | |
} | |
if (searchName) { | |
isSearchTool = tool.title.toLowerCase().includes(searchName.toLowerCase()); | |
} | |
if (isAsyncAPIOwner) { | |
isAsyncAPITool = tool.filters.isAsyncAPIOwner === isAsyncAPIOwner; | |
} | |
if (isAsyncAPIOwner) { | |
isAsyncAPITool = tool.filters.isAsyncAPIOwner === isAsyncAPIOwner; | |
} | |
if (isPaid !== 'all') { | |
if (isPaid === 'free') { | |
isPaidTool = tool.filters.hasCommercial === false; | |
} else { | |
isPaidTool = tool.filters.hasCommercial === true; | |
} | |
} | |
if (isPaid !== 'all') { | |
if (isPaid === 'free') { | |
isPaidTool = tool.filters.hasCommercial === false; | |
} else { | |
isPaidTool = tool.filters.hasCommercial === true; | |
} | |
} | |
return isLanguageTool && isTechnologyTool && isSearchTool && isAsyncAPITool && isPaidTool; | |
}, [languages, technologies, searchName, isAsyncAPIOwner, isPaid]); | |
const checkToolFilters = useCallback( | |
(tool: any) => { | |
let isLanguageTool = true; | |
let isTechnologyTool = true; | |
let isSearchTool = true; | |
let isAsyncAPITool = true; | |
let isPaidTool = true; | |
if (languages.length) { | |
isLanguageTool = false; | |
for (const language of languages) { | |
if (tool?.filters?.language?.find((item: any) => item.name === language)) { | |
isLanguageTool = true; | |
} | |
} | |
} | |
if (technologies.length) { | |
isTechnologyTool = false; | |
for (const technology of technologies) { | |
if (tool?.filters?.technology?.find((item: any) => item.name === technology)) { | |
isTechnologyTool = true; | |
} | |
} | |
} | |
if (searchName) { | |
isSearchTool = tool.title.toLowerCase().includes(searchName.toLowerCase()); | |
} | |
if (isAsyncAPIOwner) { | |
isAsyncAPITool = tool.filters.isAsyncAPIOwner === isAsyncAPIOwner; | |
} | |
if (isPaid !== 'all') { | |
if (isPaid === 'free') { | |
isPaidTool = tool.filters.hasCommercial === false; | |
} else { | |
isPaidTool = tool.filters.hasCommercial === true; | |
} | |
} | |
return isLanguageTool && isTechnologyTool && isSearchTool && isAsyncAPITool && isPaidTool; | |
}, | |
[languages, technologies, searchName, isAsyncAPIOwner, isPaid] | |
); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 83-83: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 92-92: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 ESLint
[error] 73-73: Insert ⏎····
(prettier/prettier)
[error] 74-74: Replace ····
with ······
(prettier/prettier)
[error] 75-75: Insert ··
(prettier/prettier)
[error] 76-76: Insert ··
(prettier/prettier)
[error] 77-77: Insert ··
(prettier/prettier)
[error] 78-78: Insert ··
(prettier/prettier)
[error] 80-80: Insert ··
(prettier/prettier)
[error] 81-81: Insert ··
(prettier/prettier)
[error] 82-82: Insert ··
(prettier/prettier)
[error] 83-83: Replace ········
with ··········
(prettier/prettier)
[error] 84-84: Insert ··
(prettier/prettier)
[error] 85-85: Insert ··
(prettier/prettier)
[error] 86-86: Insert ··
(prettier/prettier)
[error] 87-87: Replace ····
with ······
(prettier/prettier)
[error] 89-89: Insert ··
(prettier/prettier)
[error] 90-90: Replace ······
with ········
(prettier/prettier)
[error] 91-91: Insert ··
(prettier/prettier)
[error] 92-92: Replace ········
with ··········
(prettier/prettier)
[error] 93-93: Insert ··
(prettier/prettier)
[error] 94-94: Insert ··
(prettier/prettier)
[error] 95-95: Insert ··
(prettier/prettier)
[error] 96-96: Replace ····
with ······
(prettier/prettier)
[error] 98-98: Insert ··
(prettier/prettier)
[error] 99-99: Insert ··
(prettier/prettier)
[error] 100-100: Insert ··
(prettier/prettier)
[error] 102-102: Insert ··
(prettier/prettier)
[error] 103-103: Insert ··
(prettier/prettier)
[error] 104-104: Insert ··
(prettier/prettier)
[error] 106-106: Insert ··
(prettier/prettier)
[error] 107-107: Insert ··
(prettier/prettier)
[error] 108-108: Insert ··
(prettier/prettier)
[error] 109-109: Insert ··
(prettier/prettier)
[error] 110-110: Insert ··
(prettier/prettier)
[error] 111-111: Insert ··
(prettier/prettier)
[error] 112-112: Replace ····
with ······
(prettier/prettier)
[error] 114-114: Insert ··
(prettier/prettier)
[error] 115-115: Replace },·[languages,·technologies,·searchName,·isAsyncAPIOwner,·isPaid]
with ··},⏎····[languages,·technologies,·searchName,·isAsyncAPIOwner,·isPaid]⏎··
(prettier/prettier)
Performance Optimizations: Memoization and Lazy Loading
Related Issue- #3901
FiltersDropdown Component
useCallback
to:handleClickOption
functionuseMemo
to:FiltersDisplay Component
useCallback
to:handleClickOption
functionuseMemo
to:ToolsDashboard Component
useCallback
to:handleClickOutsideFilter
handleClickOutsideCategory
filterToolsByCategory
checkToolFilters
useMemo
to:toolsList
computationImage Loading Optimizations
loading="lazy"
attribute to:Summary by CodeRabbit