Fix React Hooks ESLint warnings in Filters and AnnouncementHero components#5126
Fix React Hooks ESLint warnings in Filters and AnnouncementHero components#5126luckyxhq wants to merge 2 commits intoasyncapi:masterfrom
Conversation
- Add missing setOpen dependency to useEffect in Filters.tsx - Remove unnecessary banners dependency from useMemo in AnnouncementHero.tsx - Apply prettier formatting to modified files Fixes asyncapi#5125
luckyxhq
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg,
princerajpoot20 and
sambhavgupta0705
as code owners
❌ Deploy Preview for asyncapi-website failed.Built without sensitive environment variables
|
|
We require all PRs to follow Conventional Commits specification. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
📝 WalkthroughWalkthroughThe PR addresses React Hooks ESLint warnings by fixing dependency arrays in two components and updating configuration formatting. Changes include removing an unnecessary constant dependency from useMemo, adding a missing state setter dependency to useEffect, introducing an early return guard for empty banner lists, and minor formatting updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/campaigns/AnnouncementHero.tsx (1)
49-62:⚠️ Potential issue | 🟡 MinorGuard against division by zero when
numberOfVisibleBannersis 0.The
useEffecton line 49 runs before the early return on line 60. WhennumberOfVisibleBannersis 0, the interval fires(index + 1) % 0, which producesNaNand silently corruptsactiveIndex. Although the early return prevents visible breakage with the current static data, this is fragile.Either move the early-return guard before all hooks (not possible per Rules of Hooks), or add a guard inside the effect:
Proposed fix
useEffect(() => { + if (numberOfVisibleBanners === 0) return; const interval = setInterval( () => setActiveIndex((index) => (index + 1) % numberOfVisibleBanners), 10000, ); return () => { clearInterval(interval); }; }, [numberOfVisibleBanners]);
🤖 Fix all issues with AI agents
In `@components/campaigns/AnnouncementHero.tsx`:
- Around line 21-30: The file AnnouncementHero has many formatting violations
(JSX attributes using double quotes, unwanted trailing commas, and
multiline/inline inconsistencies) causing CI failures; to fix, run the project's
formatter (e.g., npx prettier --write components/campaigns/AnnouncementHero.tsx)
and ensure JSX uses single quotes and Prettier/ESLint rules are respected around
the useMemo block that defines visibleBanners, the AnnouncementHero component
props, and other JSX elements; after formatting, re-commit the updated file so
that the code around AnnouncementHero, visibleBanners, banners, and
shouldShowBanner matches the repo style.
In `@config/3.0.0.json`:
- Line 15: The JSON contains an inline comment after the "properties" key which
invalidates the file; remove the comment text beginning with "#" so the line
becomes a valid JSON key ("properties": { ), or relocate the explanatory note to
external documentation (README) instead; search for the "properties" key in the
shown diff to find and fix the offending inline comment.
| export default function AnnouncementHero({ | ||
| className = '', | ||
| small = false, | ||
| }: IAnnouncementHeroProps) { | ||
| const [activeIndex, setActiveIndex] = useState(0); | ||
|
|
||
| const visibleBanners = useMemo(() => banners.filter((banner) => shouldShowBanner(banner.cfpDeadline)), [banners]); | ||
| const visibleBanners = useMemo( | ||
| () => banners.filter((banner) => shouldShowBanner(banner.cfpDeadline)), | ||
| [], | ||
| ); |
There was a problem hiding this comment.
CI is failing: formatting does not match project Prettier/ESLint config.
The pipeline reports 20+ errors across this file. Two recurring categories:
- Double quotes → single quotes: The project enforces single quotes for JSX attributes (e.g.,
'text-white'not"text-white"). - Trailing commas & multiline expansion: The Prettier config expects inline formatting and no trailing commas in many of these positions.
Please run npx prettier --write components/campaigns/AnnouncementHero.tsx (or the project's configured formatter) and re-commit to fix all formatting violations in one pass.
Also applies to: 34-36, 40-42, 50-53, 65-66, 73-73, 76-77, 101-101, 119-119
🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[error] 21-21: prettier/prettier: Replace ⏎··className·=·'',⏎··small·=·false,⏎ with ·className·=·'',·small·=·false·
[error] 23-23: prettier/prettier: Unexpected trailing comma. comma-dangle
[error] 27-27: prettier/prettier: Replace ⏎····()·=>·banners.filter((banner)·=>·shouldShowBanner(banner.cfpDeadline)),⏎····[],⏎·· with ()·=>·banners.filter((banner)·=>·shouldShowBanner(banner.cfpDeadline)),·[]
[error] 29-29: prettier/prettier: Unexpected trailing comma. comma-dangle
🤖 Prompt for AI Agents
In `@components/campaigns/AnnouncementHero.tsx` around lines 21 - 30, The file
AnnouncementHero has many formatting violations (JSX attributes using double
quotes, unwanted trailing commas, and multiline/inline inconsistencies) causing
CI failures; to fix, run the project's formatter (e.g., npx prettier --write
components/campaigns/AnnouncementHero.tsx) and ensure JSX uses single quotes and
Prettier/ESLint rules are respected around the useMemo block that defines
visibleBanners, the AnnouncementHero component props, and other JSX elements;
after formatting, re-commit the updated file so that the code around
AnnouncementHero, visibleBanners, banners, and shouldShowBanner matches the repo
style.
| } | ||
| }, | ||
| "properties": { | ||
| "properties": { #there the properties change i create |
There was a problem hiding this comment.
Remove inline comment to keep JSON valid.
# is not valid in JSON and will cause schema parsing to fail. Please remove the inline comment or move the note elsewhere (e.g., README or a separate doc).
🔧 Proposed fix
- "properties": { `#there` the properties change i create
+ "properties": {📝 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.
| "properties": { #there the properties change i create | |
| "properties": { |
🧰 Tools
🪛 Biome (2.3.13)
[error] 15-15: unexpected character #
(parse)
🤖 Prompt for AI Agents
In `@config/3.0.0.json` at line 15, The JSON contains an inline comment after the
"properties" key which invalidates the file; remove the comment text beginning
with "#" so the line becomes a valid JSON key ("properties": { ), or relocate
the explanatory note to external documentation (README) instead; search for the
"properties" key in the shown diff to find and fix the offending inline comment.
2 participants
Description
This PR fixes React Hooks ESLint warnings found in two components that could lead to potential bugs or code quality issues.
Changes Made
1.
components/dashboard/table/Filters.tsxsetOpendependency to theuseEffecthook inuseOutsideAlerter2.
components/campaigns/AnnouncementHero.tsxbannersdependency fromuseMemohookbannersis an imported constant that never changes, so including it was unnecessary and could cause needless re-computationsWhy These Changes Matter
Testing
npm run lintto verify the warnings are resolvedRelated Issue
Fixes #5125
Summary by CodeRabbit
Release Notes
Bug Fixes
Style