-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: Remove trpc/react dependency from @calcom/atoms
#23
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
base: coderabbit_combined_20260121_augment_sentry_coderabbit_1_base_refactor_remove_trpc_react_dependency_from_calcom_atoms_pr722
Are you sure you want to change the base?
Conversation
WalkthroughThese changes refactor hook logic between platform and web modules to reduce circular dependencies. A new hook is added to the web layer, platform utilities are simplified and exported, type references are updated to use repository types instead of TRPC outputs, and linter rules are configured to prevent future circular dependency violations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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
Comment |
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/platform/atoms/event-types/hooks/useTeamMembersWithSegmentPlatform.tsx (1)
13-13: Remove unusedtrpcimport that violates the new lint rule.This import from
@calcom/trpc/reactis unused in this file (the hook uses@tanstack/react-query'suseQuerydirectly with thehttpclient). Additionally, it violates thenoRestrictedImportsrule added in this same PR atbiome.jsonlines 338-344, which will cause lint errors.🐛 Proposed fix
-import { trpc } from "@calcom/trpc/react"; - import { useAtomsContext } from "../../hooks/useAtomsContext";
🤖 Fix all issues with AI agents
In `@apps/web/modules/event-types/components/EditWeightsForAllTeamMembers.tsx`:
- Around line 304-309: The list rendering in EditWeightsForAllTeamMembers
incorrectly maps over teamMembers, bypassing the filteredMembers memo and
breaking search/filter/weight merging; update the render to map over
filteredMembers (which already accounts for assignAllTeamMembers and
searchQuery) and ensure each item passed to TeamMemberItem reflects merged
weights from localWeights (use the same merge logic used when computing
filteredMembers) and still passes onWeightChange={handleWeightChange}; this
restores search filtering, localWeights display, and the assignAllTeamMembers
behavior.
In `@packages/platform/atoms/availability/AvailabilitySettings.tsx`:
- Line 102: The prop type for travelSchedules in AvailabilitySettings was
changed to Awaited<ReturnType<typeof
TravelScheduleRepository.findTravelSchedulesByUserId>> which is structurally
compatible but incompatible with the DateOverrideList prop type
RouterOutputs["viewer"]["travelSchedules"]["get"]; fix by making the types
identical: either change AvailabilitySettings' travelSchedules type to
RouterOutputs["viewer"]["travelSchedules"]["get"] or update DateOverrideList's
prop to accept Awaited<ReturnType<typeof
TravelScheduleRepository.findTravelSchedulesByUserId>> (or a shared alias/type
exported and used by both), referencing the symbols AvailabilitySettings,
travelSchedules, DateOverrideList,
RouterOutputs["viewer"]["travelSchedules"]["get"], and
TravelScheduleRepository.findTravelSchedulesByUserId so the compiler sees the
same declared type.
🧹 Nitpick comments (2)
apps/web/modules/event-types/components/EditWeightsForAllTeamMembers.tsx (1)
128-134:isPendingis destructured but never used.Consider either displaying a loading indicator while the query is pending, or removing the unused variable to avoid confusion.
apps/web/modules/event-types/hooks/useTeamMembersWithSegment.tsx (1)
1-1: Remove unuseduseQueryimport.The
useQueryimport from@tanstack/react-queryis not used; this hook usestrpc.*.useQueryinstead.♻️ Proposed fix
-import { useQuery } from "@tanstack/react-query"; import type { AttributesQueryValue } from "@calcom/lib/raqb/types";
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/modules/event-types/components/EditWeightsForAllTeamMembers.tsxapps/web/modules/event-types/hooks/useTeamMembersWithSegment.tsxbiome.jsonpackages/platform/atoms/availability/AvailabilitySettings.tsxpackages/platform/atoms/event-types/hooks/useTeamMembersWithSegmentPlatform.tsxpackages/platform/atoms/package.jsonpackages/platform/atoms/tsconfig.json
💤 Files with no reviewable changes (1)
- packages/platform/atoms/tsconfig.json
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/modules/event-types/hooks/useTeamMembersWithSegment.tsx (2)
packages/platform/atoms/event-types/hooks/useTeamMembersWithSegmentPlatform.tsx (2)
UseTeamMembersWithSegmentProps(18-25)useProcessTeamMembersData(27-64)packages/app-store/routing-forms/types/types.d.ts (1)
AttributesQueryValue(113-113)
packages/platform/atoms/availability/AvailabilitySettings.tsx (1)
packages/features/travelSchedule/repositories/TravelScheduleRepository.ts (1)
TravelScheduleRepository(3-19)
🔇 Additional comments (5)
packages/platform/atoms/package.json (1)
75-75: LGTM!The new export alias follows the established pattern for exposing hooks from the atoms package.
biome.json (1)
326-358: LGTM!The lint rule effectively enforces the architectural boundary by preventing both absolute (
@calcom/trpc/react) and relative (../../trpc/react) imports, which is essential for maintaining the decoupling objective of this refactor.packages/platform/atoms/event-types/hooks/useTeamMembersWithSegmentPlatform.tsx (1)
66-108: LGTM!The platform hook correctly uses
@tanstack/react-querydirectly with the HTTP client instead of tRPC, aligning with the PR objective. The query is properly guarded byisInitbefore making requests.apps/web/modules/event-types/hooks/useTeamMembersWithSegment.tsx (1)
25-30: LGTM!The transformation correctly adapts the tRPC response shape to match the expected format for
useProcessTeamMembersData. The web hook properly delegates data processing to the shared platform utility while handling its own data fetching via tRPC.packages/platform/atoms/availability/AvailabilitySettings.tsx (1)
32-32: LGTM!The type import change from
RouterOutputstoTravelScheduleRepositoryaligns with the refactor goal of removing tRPC dependencies from the atoms package. Using the repository's return type directly is more explicit and maintains type safety.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| {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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical regression: Rendering bypasses search filter and weight merging.
The list now renders teamMembers directly instead of filteredMembers, which breaks:
- Search functionality -
searchQueryfiltering is no longer applied - Weight display -
localWeightsvalues are not merged into the rendered members - Member filtering - The
assignAllTeamMemberscondition (lines 242-248) is bypassed
The filteredMembers memo (lines 231-249) is still computed but unused.
🐛 Proposed fix to restore filteredMembers usage
<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>📝 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.
| {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> | |
| )} | |
| {filteredMembers.map((member) => ( | |
| <TeamMemberItem key={member.value} member={member} onWeightChange={handleWeightChange} /> | |
| ))} | |
| {filteredMembers.length === 0 && ( | |
| <div className="text-subtle py-4 text-center text-sm">{t("no_members_found")}</div> | |
| )} |
🤖 Prompt for AI Agents
In `@apps/web/modules/event-types/components/EditWeightsForAllTeamMembers.tsx`
around lines 304 - 309, The list rendering in EditWeightsForAllTeamMembers
incorrectly maps over teamMembers, bypassing the filteredMembers memo and
breaking search/filter/weight merging; update the render to map over
filteredMembers (which already accounts for assignAllTeamMembers and
searchQuery) and ensure each item passed to TeamMemberItem reflects merged
weights from localWeights (use the same merge logic used when computing
filteredMembers) and still passes onWeightChange={handleWeightChange}; this
restores search filtering, localWeights display, and the assignAllTeamMembers
behavior.
| skeletonLabel?: string; | ||
| schedule: AvailabilitySettingsScheduleType; | ||
| travelSchedules?: RouterOutputs["viewer"]["travelSchedules"]["get"]; | ||
| travelSchedules?: Awaited<ReturnType<typeof TravelScheduleRepository.findTravelSchedulesByUserId>>; |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all usages of AvailabilitySettings component
rg -n '<AvailabilitySettings' --type=tsx -g '!**/AvailabilitySettings.tsx'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 113
🏁 Script executed:
#!/bin/bash
# Find the AvailabilitySettings file and examine lines 102 and 198
fd -e tsx -p '*AvailabilitySettings.tsx' | head -5Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 390
🏁 Script executed:
#!/bin/bash
# Count lines to understand file size before reading
wc -l packages/platform/atoms/availability/AvailabilitySettings.tsxRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Read the AvailabilitySettings file to see lines 102 and 198
sed -n '95,110p; 190,205p' packages/platform/atoms/availability/AvailabilitySettings.tsxRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 994
🏁 Script executed:
#!/bin/bash
# Find all usages of AvailabilitySettings component - search for the component name being used
rg -n '<AvailabilitySettings' --type=ts | head -20Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 888
🏁 Script executed:
#!/bin/bash
# Find where travelSchedules prop is passed to AvailabilitySettings
rg -n 'travelSchedules' packages/platform/atoms/availability/ -A2 -B2Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 3178
🏁 Script executed:
#!/bin/bash
# Find TravelScheduleRepository to understand the return type
fd -name '*TravelScheduleRepository*'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 316
🏁 Script executed:
#!/bin/bash
# Find TravelScheduleRepository file with corrected syntax
fd 'TravelScheduleRepository'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 159
🏁 Script executed:
#!/bin/bash
# Check the consumer files to see what types they pass for travelSchedules
cat -n packages/platform/atoms/availability/wrappers/AvailabilitySettingsPlatformWrapper.tsx | sed -n '145,160p'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 552
🏁 Script executed:
#!/bin/bash
# Check another consumer file
cat -n apps/web/modules/availability/[schedule]/schedule-view.tsx | sed -n '100,120p'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 909
🏁 Script executed:
#!/bin/bash
# Search for the old type RouterOutputs to see if it's still used
rg 'RouterOutputs.*travelSchedules.*get'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 473
🏁 Script executed:
#!/bin/bash
# Find where TravelScheduleRepository.findTravelSchedulesByUserId is used/called
rg 'findTravelSchedulesByUserId'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 933
🏁 Script executed:
#!/bin/bash
# Check the handler to see what it returns
cat -n packages/trpc/server/routers/viewer/travelSchedules/getTravelSchedules.handler.tsRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 617
🏁 Script executed:
#!/bin/bash
# Check schedule-view.tsx more broadly to understand types
cat -n apps/web/modules/availability/[schedule]/schedule-view.tsx | sed -n '1,50p'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 2619
🏁 Script executed:
#!/bin/bash
# Check what RouterOutputs["viewer"]["travelSchedules"]["get"] is
rg -A5 '"viewer".*travelSchedules' --type=ts | head -30Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 1763
🏁 Script executed:
#!/bin/bash
# Check TravelScheduleRepository.findTravelSchedulesByUserId implementation
cat -n packages/features/travelSchedule/repositories/TravelScheduleRepository.tsRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 632
🏁 Script executed:
#!/bin/bash
# Check how the TRPC route is structured
fd '*travelSchedules*' -t f | grep -E '(router|procedures)'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 382
🏁 Script executed:
#!/bin/bash
# Look for the TRPC route definition for travelSchedules
rg 'travelSchedules.*procedure' -A5Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 68
🏁 Script executed:
#!/bin/bash
# Look for the TRPC router structure
fd -name '*.ts' --type f -path '*/routers/viewer/*' | grep -i travelRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 316
🏁 Script executed:
#!/bin/bash
# Check the TRPC router definition for travelSchedules
rg -A10 'travelSchedules.*:' packages/trpc/server/routers/viewer/ | head -30Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 2267
🏁 Script executed:
#!/bin/bash
# Check if there are any TS compilation errors or type mismatches by looking at type definitions
cat -n packages/trpc/react/index.ts | sed -n '1,50p'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 263
🏁 Script executed:
#!/bin/bash
# Find the travelSchedules router
fd 'travelSchedules' --type f -path '*/routers/viewer/*'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 254
🏁 Script executed:
#!/bin/bash
# Check the structure of the travelSchedules router
find packages/trpc/server/routers/viewer -name '*travelSchedule*' -type fRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 68
🏁 Script executed:
#!/bin/bash
# Look at the structure of the handler being exported
rg -B5 -A15 'getTravelSchedulesHandler' packages/trpc/server/routers/viewer/travelSchedules/Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 1889
🏁 Script executed:
#!/bin/bash
# Check the procedure definition in the router
cat packages/trpc/server/routers/viewer/travelSchedules/getTravelSchedules.handler.ts
cat packages/trpc/server/routers/viewer/travelSchedules/getTravelSchedules.ts 2>/dev/null || echo "File not found, checking for other files:"
find packages/trpc/server/routers/viewer/travelSchedules -type fRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 802
🏁 Script executed:
#!/bin/bash
# Check for any TypeScript compilation errors or type issues
# Look at the actual types that should be inferred
rg 'Awaited<ReturnType' packages/platform/atoms/availability/AvailabilitySettings.tsxRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 289
🏁 Script executed:
#!/bin/bash
# Check if schedule-view.tsx actually compiles/uses the type correctly
cat -n apps/web/modules/availability/[schedule]/schedule-view.tsx | sed -n '108,115p'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 414
🏁 Script executed:
#!/bin/bash
# Check what DateOverrideList expects (it receives travelSchedules)
cat -n packages/features/schedules/components/DateOverrideList.tsx | sed -n '1,40p'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 1799
🏁 Script executed:
#!/bin/bash
# Confirm that the schema or any other constraints don't differ
cat packages/trpc/server/routers/viewer/travelSchedules/getTravelSchedules.schema.tsRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 96
🏁 Script executed:
#!/bin/bash
# Check what AvailabilitySettings passes to DateOverrideList
cat -n packages/platform/atoms/availability/AvailabilitySettings.tsx | sed -n '240,250p'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 558
🏁 Script executed:
#!/bin/bash
# Search for where DateOverrideList receives the travelSchedules prop from AvailabilitySettings
rg -B3 -A3 'DateOverrideList' packages/platform/atoms/availability/AvailabilitySettings.tsx | head -40Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 805
🏁 Script executed:
#!/bin/bash
# Check what type the function accepts
cat -n packages/platform/atoms/availability/AvailabilitySettings.tsx | sed -n '230,260p'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 1412
🏁 Script executed:
#!/bin/bash
# Check if there's a type mismatch between what AvailabilitySettings provides and what DateOverrideList expects
# Look at the exact parameter type definition in DateOverrideList
cat -n packages/features/schedules/components/DateOverrideList.tsx | sed -n '36,42p'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 391
🏁 Script executed:
#!/bin/bash
# Verify the actual usage - does AvailabilitySettings pass the prop directly?
# Check the exact line where travelSchedules is passed from AvailabilitySettings to DateOverrideList
rg -n 'travelSchedules=' packages/platform/atoms/availability/AvailabilitySettings.tsxRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 193
🏁 Script executed:
#!/bin/bash
# Check if there are any type assertions or workarounds in AvailabilitySettings
cat -n packages/platform/atoms/availability/AvailabilitySettings.tsx | sed -n '100,115p'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 765
Type mismatch requires fixing: DateOverrideList incompatibility.
The prop type change at line 102 creates a type compatibility issue. AvailabilitySettings now declares travelSchedules as Awaited<ReturnType<typeof TravelScheduleRepository.findTravelSchedulesByUserId>>, but passes it to DateOverrideList (line 246), which expects RouterOutputs["viewer"]["travelSchedules"]["get"].
Although both types resolve to the same runtime shape, TypeScript treats them as different types. Either align DateOverrideList to accept the repository type, or use RouterOutputs["viewer"]["travelSchedules"]["get"] consistently in AvailabilitySettings.
🤖 Prompt for AI Agents
In `@packages/platform/atoms/availability/AvailabilitySettings.tsx` at line 102,
The prop type for travelSchedules in AvailabilitySettings was changed to
Awaited<ReturnType<typeof TravelScheduleRepository.findTravelSchedulesByUserId>>
which is structurally compatible but incompatible with the DateOverrideList prop
type RouterOutputs["viewer"]["travelSchedules"]["get"]; fix by making the types
identical: either change AvailabilitySettings' travelSchedules type to
RouterOutputs["viewer"]["travelSchedules"]["get"] or update DateOverrideList's
prop to accept Awaited<ReturnType<typeof
TravelScheduleRepository.findTravelSchedulesByUserId>> (or a shared alias/type
exported and used by both), referencing the symbols AvailabilitySettings,
travelSchedules, DateOverrideList,
RouterOutputs["viewer"]["travelSchedules"]["get"], and
TravelScheduleRepository.findTravelSchedulesByUserId so the compiler sees the
same declared type.
Benchmark PR from qodo-benchmark#722
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.