refactor: Remove trpc/server dependency from @calcom/atoms#94
Conversation
Code Review by Qodo
1. validateCreateScheduleInput throws generic Error
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+38
to
+47
| export function validateCreateScheduleInput(input: unknown): CreateScheduleInput { | ||
| if (!input || typeof input !== 'object') { | ||
| throw new Error('Invalid input: must be an object'); | ||
| } | ||
|
|
||
| const data = input as Record<string, unknown>; | ||
|
|
||
| if (typeof data.name !== 'string' || data.name.length === 0) { | ||
| throw new Error('Invalid input: name must be a non-empty string'); | ||
| } |
There was a problem hiding this comment.
1. validatecreatescheduleinput throws generic error 📘 Rule violation ⛯ Reliability
• The new validateCreateScheduleInput uses throw new Error(...) for validation failures, which provides no typed error code or structured context for downstream handling. • This makes it harder to categorize, monitor, and map failures to appropriate responses, contrary to the required hierarchical error structure.
Agent prompt
## Issue description
`validateCreateScheduleInput` throws generic `Error` objects, which violates the requirement for hierarchical custom errors with typed codes and structured context.
## Issue Context
This validation utility will be used to reject invalid inputs; without typed codes/context, callers cannot reliably categorize and handle the failure.
## Fix Focus Areas
- packages/platform/atoms/hooks/schedules/types.ts[38-47]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+32
to
+49
| export type CreateScheduleInput = { | ||
| name: string; | ||
| schedule?: { start: Date; end: Date }[][]; | ||
| eventTypeId?: number; | ||
| }; | ||
|
|
||
| export function validateCreateScheduleInput(input: unknown): CreateScheduleInput { | ||
| if (!input || typeof input !== 'object') { | ||
| throw new Error('Invalid input: must be an object'); | ||
| } | ||
|
|
||
| const data = input as Record<string, unknown>; | ||
|
|
||
| if (typeof data.name !== 'string' || data.name.length === 0) { | ||
| throw new Error('Invalid input: name must be a non-empty string'); | ||
| } | ||
|
|
||
| return data as CreateScheduleInput; |
There was a problem hiding this comment.
2. validatecreatescheduleinput returns cast 📘 Rule violation ⛨ Security
• The new validation only checks name and then returns data as CreateScheduleInput, leaving schedule and eventTypeId effectively unvalidated. • This allows malformed external input to flow through as trusted types, increasing the risk of runtime errors and unsafe data handling.
Agent prompt
## Issue description
The validation helper returns an unsafe cast (`data as CreateScheduleInput`) after only partially validating the input.
## Issue Context
`input` is `unknown` and should be treated as external/untrusted; `schedule` and `eventTypeId` need explicit validation to prevent malformed data from being treated as valid.
## Fix Focus Areas
- packages/platform/atoms/hooks/schedules/types.ts[32-49]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+16
to
+29
| export type GetAvailabilityListHandlerReturn = { | ||
| schedules: (Omit<Schedule, "userId"> & { | ||
| availability: { | ||
| id: number; | ||
| userId: number | null; | ||
| eventTypeId: number | null; | ||
| days: string[]; | ||
| startTime: Date; | ||
| endTime: Date; | ||
| date: Date | null; | ||
| scheduleId: number | null; | ||
| }[]; | ||
| isDefault: boolean; | ||
| })[]; |
There was a problem hiding this comment.
3. Schedule days type wrong 🐞 Bug ✓ Correctness
• GetAvailabilityListHandlerReturn in atoms defines availability.days as string[], but the persisted/API shape uses numeric weekday values (Int[]). • This can cause TS incompatibilities when passing schedules into existing schedule transformers/components that expect days: number[]. • Because these types are now “hand-copied” into atoms, they are likely to drift further from the server contract over time.
Agent prompt
### Issue description
`packages/platform/atoms/hooks/schedules/types.ts` defines `availability.days` as `string[]`, but the real persisted/API shape uses numeric weekday values (`Int[]`). This mismatch breaks type compatibility with existing consumers (e.g., `getScheduleListItemData`) and is likely to drift.
### Issue Context
The atoms package is being decoupled from `@calcom/trpc` by recreating server types locally. These new local types must still match the actual REST response served by the atoms API.
### Fix Focus Areas
- packages/platform/atoms/hooks/schedules/types.ts[16-29]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+17
to
+23
| export type Slot = { | ||
| time: string; | ||
| userIds?: string[]; | ||
| attendees?: number; | ||
| bookingUid?: string; | ||
| users?: string[]; | ||
| }; |
There was a problem hiding this comment.
4. Slot userids type drift 🐞 Bug ✓ Correctness
• The new atoms Slot type defines userIds?: string[], but the server-side slot pipeline uses number[] for userIds and can include it in the slots returned to clients. • This mismatch can cause subtle bugs when consumers treat userIds numerically (comparisons, lookups) or when TS types are used to derive other contracts. • It also increases the risk that different parts of the system will disagree on what a “slot” contains.
Agent prompt
### Issue description
Atoms `Slot.userIds` is typed as `string[]`, but server slot generation uses numeric IDs (`number[]`) and can include them in slot objects returned to clients.
### Issue Context
This PR removes atoms’ imports from TRPC by redefining types locally. Local types must still mirror the actual REST responses.
### Fix Focus Areas
- packages/platform/atoms/booker/types.ts[17-23]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+115
to
+128
| type SlotInfo = { | ||
| time: string; | ||
| attendees?: number; | ||
| bookingUid?: string; | ||
| away?: boolean; | ||
| fromUser?: { | ||
| id: string; | ||
| displayName: string | null; | ||
| }; | ||
| toUser?: { | ||
| id: number; | ||
| username: string | null; | ||
| displayName: string | null; | ||
| }; |
There was a problem hiding this comment.
5. Slotinfo fromuser id wrong 🐞 Bug ✓ Correctness
• SlotInfo.fromUser.id is typed as string, but server-side IFromUser.id is number. • This creates inconsistent ID typing inside the same response shape (fromUser.id is string while toUser.id is number), increasing the chance of runtime bugs and incorrect client-side comparisons. • The mismatch is especially risky in code paths dealing with out-of-office routing/host delegation where these IDs may be used to correlate users.
Agent prompt
### Issue description
Atoms `SlotInfo.fromUser.id` is typed as `string`, but server-side `IFromUser.id` is numeric.
### Issue Context
This type is part of `GetAvailableSlotsResponse` and will be consumed by atoms clients. Mixed ID types (`fromUser.id` vs `toUser.id`) are error-prone.
### Fix Focus Areas
- packages/platform/atoms/booker/types.ts[115-128]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| "./add-members-switch/AddMembersWithSwitchPlatformWrapper": "./add-members-switch/AddMembersWithSwitchPlatformWrapper.tsx", | ||
| "./availability/AvailabilitySettings": "./availability/AvailabilitySettings.tsx", | ||
| "./booker": "./booker/index.ts", | ||
| "./booker/types": "./booker/types.ts", |
There was a problem hiding this comment.
6. Removed booker subpath export 🐞 Bug ⛯ Reliability
• @calcom/atoms export map drops the ./booker subpath and only exports ./booker/types. • While there are no in-repo imports of @calcom/atoms/booker, this is a breaking change for any external consumers relying on that path. • If this package is published (it has a version and npm scripts), this should be treated as a compatibility break (restore export or bump major).
Agent prompt
### Issue description
`packages/platform/atoms/package.json` removed the `./booker` export, which can break external consumers relying on that subpath.
### Issue Context
Even if the monorepo doesn’t currently use this path, package exports are part of the published contract.
### Fix Focus Areas
- packages/platform/atoms/package.json[55-60]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Benchmark PR from qodo-benchmark#716