refactor: Remove trpc/react dependency from @calcom/atoms#12
Conversation
📝 WalkthroughWalkthroughThe PR restructures hook dependencies and import organization across the web and atoms packages. It migrates Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 (2)
packages/platform/atoms/event-types/hooks/useTeamMembersWithSegmentPlatform.tsx (2)
13-13: Remove unusedtrpcimport.This import is unused in the file and contradicts the PR's objective of removing
@calcom/trpc/reactdependency from the atoms package. Additionally, the new biome.json linter rule will flag this import as an error.🔧 Proposed fix
-import { trpc } from "@calcom/trpc/react"; -
83-94: Handle potentially undefinederror.messagein error response.Line 90 accesses
res.data.error.messagewhen the API response status is notSUCCESS_STATUS. However, according to theErrorTypedefinition, themessageproperty is optional (message?: string), so it could beundefined. Throwingnew Error(undefined)results in an error message of the string"undefined", which is not informative. Provide a fallback message or accesserror.codeas a fallback.
🤖 Fix all issues with AI agents
In `@apps/web/modules/event-types/components/EditWeightsForAllTeamMembers.tsx`:
- Around line 304-309: The list rendering currently maps over teamMembers,
bypassing the search filter; change the rendering to map over filteredMembers so
the searchQuery/TextField filtering is applied. Locate the JSX where
TeamMemberItem is rendered (the map using teamMembers and the conditional that
checks teamMembers.length) and replace those references with filteredMembers
while keeping the TeamMemberItem key, member prop, and
onWeightChange={handleWeightChange} intact; also update the "no members"
conditional to check filteredMembers.length so the empty state reflects the
filtered results.
In `@apps/web/modules/event-types/hooks/useTeamMembersWithSegment.tsx`:
- Line 1: Remove the unused named import useQuery from the top import line
(import { useQuery } from "@tanstack/react-query"); it's not referenced anywhere
in this module because the code uses
trpc.viewer.attributes.findTeamMembersMatchingAttributeLogic.useQuery instead,
so delete the useQuery symbol from the import to eliminate the lint error and
unused import.
🧹 Nitpick comments (1)
apps/web/modules/event-types/components/EditWeightsForAllTeamMembers.tsx (1)
126-128: Conditional hook pattern may violate React's rules of hooks.The pattern of conditionally selecting a hook and then calling it could be problematic if
isPlatformchanges during the component lifecycle. While this likely works ifisPlatformis stable (from context), consider a safer pattern.♻️ Safer alternative using both hooks with conditional enabling
- const useTeamMembersHook = isPlatform ? useTeamMembersWithSegmentPlatform : useTeamMembersWithSegment; - - const { teamMembers, localWeightsInitialValues, isPending } = useTeamMembersHook({ + const platformResult = useTeamMembersWithSegmentPlatform({ + initialTeamMembers, + assignRRMembersUsingSegment: isPlatform && assignRRMembersUsingSegment, + teamId, + queryValue, + value, + }); + + const webResult = useTeamMembersWithSegment({ initialTeamMembers, - assignRRMembersUsingSegment, + assignRRMembersUsingSegment: !isPlatform && assignRRMembersUsingSegment, teamId, queryValue, value, }); + + const { teamMembers, localWeightsInitialValues, isPending } = isPlatform ? platformResult : webResult;Alternatively, if
isPlatformis guaranteed to be stable for the component's lifetime (determined at mount and never changes), the current pattern is acceptable but should be documented with a comment.
| {teamMembers.map((member) => ( | ||
| <TeamMemberItem key={member.value} member={member} onWeightChange={handleWeightChange} /> | ||
| ))} | ||
| {filteredMembers.length === 0 && ( | ||
| {teamMembers.length === 0 && ( | ||
| <div className="text-subtle py-4 text-center text-sm">{t("no_members_found")}</div> | ||
| )} |
There was a problem hiding this comment.
Search filtering is broken - should use filteredMembers instead of teamMembers.
The rendering loop iterates over teamMembers directly, but the search functionality (TextField on lines 294-301) updates searchQuery which is used to compute filteredMembers (lines 231-249). By using teamMembers instead of filteredMembers, the search/filter functionality is broken.
🐛 Proposed fix to restore search functionality
<div className="flex max-h-[80dvh] flex-col overflow-y-auto rounded-md border">
- {teamMembers.map((member) => (
+ {filteredMembers.map((member) => (
<TeamMemberItem key={member.value} member={member} onWeightChange={handleWeightChange} />
))}
- {teamMembers.length === 0 && (
+ {filteredMembers.length === 0 && (
<div className="text-subtle py-4 text-center text-sm">{t("no_members_found")}</div>
)}
</div>🤖 Prompt for AI Agents
In `@apps/web/modules/event-types/components/EditWeightsForAllTeamMembers.tsx`
around lines 304 - 309, The list rendering currently maps over teamMembers,
bypassing the search filter; change the rendering to map over filteredMembers so
the searchQuery/TextField filtering is applied. Locate the JSX where
TeamMemberItem is rendered (the map using teamMembers and the conditional that
checks teamMembers.length) and replace those references with filteredMembers
while keeping the TeamMemberItem key, member prop, and
onWeightChange={handleWeightChange} intact; also update the "no members"
conditional to check filteredMembers.length so the empty state reflects the
filtered results.
| @@ -0,0 +1,37 @@ | |||
| import { useQuery } from "@tanstack/react-query"; | |||
There was a problem hiding this comment.
Remove unused useQuery import.
The useQuery import from @tanstack/react-query is not used in this file. The TRPC query uses trpc.viewer.attributes.findTeamMembersMatchingAttributeLogic.useQuery instead.
🧹 Proposed fix
-import { useQuery } from "@tanstack/react-query";
import type { AttributesQueryValue } from "@calcom/lib/raqb/types";📝 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.
| import { useQuery } from "@tanstack/react-query"; | |
| import type { AttributesQueryValue } from "@calcom/lib/raqb/types"; |
🤖 Prompt for AI Agents
In `@apps/web/modules/event-types/hooks/useTeamMembersWithSegment.tsx` at line 1,
Remove the unused named import useQuery from the top import line (import {
useQuery } from "@tanstack/react-query"); it's not referenced anywhere in this
module because the code uses
trpc.viewer.attributes.findTeamMembersMatchingAttributeLogic.useQuery instead,
so delete the useQuery symbol from the import to eliminate the lint error and
unused import.
2 participants
Benchmark PR from agentic-review-benchmarks#12
Summary by CodeRabbit
Bug Fixes
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.