refactor: Remove deprecated react api usage#2034
Conversation
github-actions
Bot
added
community
frontend
mfortman11
changed the title
WalkthroughThis PR modernizes React usage by converting forwardRef-based UI components and ChatInput into plain function components that accept ref as a prop, and replaces useContext calls with React's use hook across context providers. Separately, several async handlers (bulk delete, role toggle, connector checks) are refactored to run operations concurrently via Promise.all. ChangesReact 19 API Modernization
Concurrent Async Refactors
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Handler
participant QueryClient
participant API
Handler->>QueryClient: Promise.all([refresh, invalidate, refetch])
par concurrent operations
QueryClient->>API: refreshTasks / refreshPermissions
and
QueryClient->>QueryClient: invalidateQueries(search/connectors)
and
QueryClient->>API: refetchQueries / status+token checks
end
QueryClient-->>Handler: all resolved
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
React Doctor found 9 new issues in 5 files · 9 warnings · score 70 / 100 (Needs work) · 67 fixed · vs 9 warnings
Reviewed by React Doctor for commit |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // Fetch all filters once when dropdown opens | ||
| const { data: allFilters = [] } = useGetAllFiltersQuery({ | ||
| enabled: isFilterDropdownOpen, | ||
| export function ChatInput({ |
There was a problem hiding this comment.
React Doctor · react-doctor/no-giant-component (warning)
Component "ChatInput" is 591 lines long, which is hard to read & change. Split it into a few smaller components.
Fix → Pull each section into its own component so the parent is easier to read, test, and change.
| setIsFilterHighlighted, | ||
| onFileSelected, | ||
| ref, | ||
| }: ChatInputProps & { ref?: React.Ref<ChatInputHandle> }) { |
There was a problem hiding this comment.
React Doctor · react-doctor/prefer-useReducer (warning)
5 useState calls in "ChatInput" can each trigger a separate render.
Fix → Group related state in useReducer so one logical update does not fan out into separate renders.
| <div className={cn("p-4 pt-2", className)}>{children}</div> | ||
| </AccordionPrimitive.Content> | ||
| )); | ||
| function AccordionTrigger({ |
There was a problem hiding this comment.
React Doctor · react-doctor/no-multi-comp (warning)
This file declares several components, so each component is harder to find, test, and change.
Fix → Move secondary components into their own files so each component stays easier to find, test, and change.
| } | ||
|
|
||
| AccordionContent.displayName = AccordionPrimitive.Content.displayName; | ||
| function AccordionContent({ |
There was a problem hiding this comment.
React Doctor · react-doctor/no-multi-comp (warning)
This file declares several components, so each component is harder to find, test, and change.
Fix → Move secondary components into their own files so each component stays easier to find, test, and change.
| /> | ||
| )); | ||
| AvatarImage.displayName = AvatarPrimitive.Image.displayName; | ||
| function AvatarImage({ |
There was a problem hiding this comment.
React Doctor · react-doctor/no-multi-comp (warning)
This file declares several components, so each component is harder to find, test, and change.
Fix → Move secondary components into their own files so each component stays easier to find, test, and change.
| /> | ||
| )); | ||
| AvatarFallback.displayName = AvatarPrimitive.Fallback.displayName; | ||
| function AvatarFallback({ |
There was a problem hiding this comment.
React Doctor · react-doctor/no-multi-comp (warning)
This file declares several components, so each component is harder to find, test, and change.
Fix → Move secondary components into their own files so each component stays easier to find, test, and change.
| ref, | ||
| ...props | ||
| }: InputProps) { | ||
| const [hasValue, setHasValue] = React.useState( |
There was a problem hiding this comment.
React Doctor · react-doctor/rerender-state-only-in-handlers (warning)
Each update to "hasValue" redraws your component for nothing because this useState is set but never shown on screen.
Fix → Use useRef instead of useState when the value is only set and never shown on screen. ref.current = ... updates it without redrawing the component.
| /> | ||
| )); | ||
| TabsTrigger.displayName = TabsPrimitive.Trigger.displayName; | ||
| function TabsTrigger({ |
There was a problem hiding this comment.
React Doctor · react-doctor/no-multi-comp (warning)
This file declares several components, so each component is harder to find, test, and change.
Fix → Move secondary components into their own files so each component stays easier to find, test, and change.
| /> | ||
| )); | ||
| TabsContent.displayName = TabsPrimitive.Content.displayName; | ||
| function TabsContent({ |
There was a problem hiding this comment.
React Doctor · react-doctor/no-multi-comp (warning)
This file declares several components, so each component is harder to find, test, and change.
Fix → Move secondary components into their own files so each component stays easier to find, test, and change.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/app/knowledge/page.tsx (1)
794-800: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRedundant invalidate + refetch for the same query keys.
invalidateQueriesdefaults torefetchType: "active", so it already refetches activesearch/listFilesqueries and its returned promise resolves only after those refetches settle. Pairing eachinvalidateQuerieswith arefetchQuerieson the same key is redundant. You can drop the explicitrefetchQueriescalls, or userefetchType: "none"on the invalidations if the intent is to invalidate-then-refetch explicitly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/app/knowledge/page.tsx` around lines 794 - 800, The `Promise.all` block in `knowledge/page.tsx` is doing redundant cache work for the same `search` and `listFiles` keys: `queryClient.invalidateQueries` already triggers active refetches and waits for them, so the matching `refetchQueries` calls should be removed. Update the logic around `refreshTasks`, `invalidateQueries`, and `refetchQueries` so each query key is handled only once, or switch the invalidations to `refetchType: "none"` if you want to keep an explicit invalidate-then-refetch flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/components/knowledge-dropdown.tsx`:
- Around line 246-257: The handling of `connectorsResult.connectors` in
`knowledge-dropdown.tsx` is missing the same null-safe guard used elsewhere,
which can throw when the `/api/connectors` payload omits `connectors`. Update
the `availableTypes` filter and the `connectorInfo` population loop to use
optional chaining on `connectorsResult.connectors` and its indexed entries,
matching the existing access pattern around the connector type checks. This
should be fixed in the logic that builds `availableTypes` and fills
`connectorInfo` so the dropdown degrades safely when connector data is absent.
---
Nitpick comments:
In `@frontend/app/knowledge/page.tsx`:
- Around line 794-800: The `Promise.all` block in `knowledge/page.tsx` is doing
redundant cache work for the same `search` and `listFiles` keys:
`queryClient.invalidateQueries` already triggers active refetches and waits for
them, so the matching `refetchQueries` calls should be removed. Update the logic
around `refreshTasks`, `invalidateQueries`, and `refetchQueries` so each query
key is handled only once, or switch the invalidations to `refetchType: "none"`
if you want to keep an explicit invalidate-then-refetch flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c316934a-1011-4f2e-acca-7bffdfd696ca
📒 Files selected for processing (26)
frontend/app/chat/_components/chat-input.tsxfrontend/app/knowledge/page.tsxfrontend/components/dev-role-toggle.tsxfrontend/components/knowledge-dropdown.tsxfrontend/components/ui/accordion.tsxfrontend/components/ui/avatar.tsxfrontend/components/ui/banner.tsxfrontend/components/ui/button.tsxfrontend/components/ui/card.tsxfrontend/components/ui/command.tsxfrontend/components/ui/dialog.tsxfrontend/components/ui/dropdown-menu.tsxfrontend/components/ui/input.tsxfrontend/components/ui/popover.tsxfrontend/components/ui/scroll-area.tsxfrontend/components/ui/select.tsxfrontend/components/ui/separator.tsxfrontend/components/ui/slider.tsxfrontend/components/ui/switch.tsxfrontend/components/ui/tabs.tsxfrontend/components/ui/tooltip.tsxfrontend/contexts/auth-context.tsxfrontend/contexts/brand-context.tsxfrontend/contexts/chat-context.tsxfrontend/contexts/knowledge-filter-context.tsxfrontend/contexts/task-context.tsx
| const availableTypes = cloudConnectorTypes.filter( | ||
| (type) => connectorsResult.connectors[type], | ||
| ); | ||
|
|
||
| for (const type of availableTypes) { | ||
| connectorInfo[type] = { | ||
| name: connectorsResult.connectors[type].name, | ||
| available: connectorsResult.connectors[type].available, | ||
| connected: false, | ||
| hasToken: false, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Inconsistent null-safety on connectorsResult.connectors.
At Line 227 you defensively read connectorsResult.connectors?.[d.connectorType]?.available, but here connectorsResult.connectors[type] (Lines 247, 252, 253) omits the guard. If connectors is absent from the /api/connectors payload, the .filter at Line 246 throws, and the outer catch swallows it — silently disabling all cloud connector entries. Apply the same optional chaining for consistency.
🛡️ Proposed guard
- const availableTypes = cloudConnectorTypes.filter(
- (type) => connectorsResult.connectors[type],
- );
+ const availableTypes = cloudConnectorTypes.filter(
+ (type) => connectorsResult.connectors?.[type],
+ );
for (const type of availableTypes) {
connectorInfo[type] = {
- name: connectorsResult.connectors[type].name,
- available: connectorsResult.connectors[type].available,
+ name: connectorsResult.connectors[type]?.name,
+ available: connectorsResult.connectors[type]?.available,
connected: false,
hasToken: false,
};
}📝 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 availableTypes = cloudConnectorTypes.filter( | |
| (type) => connectorsResult.connectors[type], | |
| ); | |
| for (const type of availableTypes) { | |
| connectorInfo[type] = { | |
| name: connectorsResult.connectors[type].name, | |
| available: connectorsResult.connectors[type].available, | |
| connected: false, | |
| hasToken: false, | |
| }; | |
| } | |
| const availableTypes = cloudConnectorTypes.filter( | |
| (type) => connectorsResult.connectors?.[type], | |
| ); | |
| for (const type of availableTypes) { | |
| connectorInfo[type] = { | |
| name: connectorsResult.connectors[type]?.name, | |
| available: connectorsResult.connectors[type]?.available, | |
| connected: false, | |
| hasToken: false, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/components/knowledge-dropdown.tsx` around lines 246 - 257, The
handling of `connectorsResult.connectors` in `knowledge-dropdown.tsx` is missing
the same null-safe guard used elsewhere, which can throw when the
`/api/connectors` payload omits `connectors`. Update the `availableTypes` filter
and the `connectorInfo` population loop to use optional chaining on
`connectorsResult.connectors` and its indexed entries, matching the existing
access pattern around the connector type checks. This should be fixed in the
logic that builds `availableTypes` and fills `connectorInfo` so the dropdown
degrades safely when connector data is absent.
1 participant
no-react19-deprecated-apis
useContext→use()(auth-context,brand-context,chat-context,knowledge-filter-context,task-context,banner)forwardRefwrappers,refis now a regular prop (accordion,avatar,button,card,command,dialog,dropdown-menu,input,popover,scroll-area,select,separator,slider,switch, tabs, tooltip)chat-input.tsxSummary by CodeRabbit