fix: type error in the EventAvailabilityTabPlatformWrapper component#21983
fix: type error in the EventAvailabilityTabPlatformWrapper component#21983keithwillcode merged 7 commits intocalcom:mainfrom
EventAvailabilityTabPlatformWrapper component#21983Conversation
|
@apsinghdev is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
apsinghdev
changed the title
EventAvailabilityTabPlatformWrapper componentEventAvailabilityTabPlatformWrapper component
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (06/23/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (06/23/25)1 label was added to this PR based on Keith Williams's automation. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic found 1 issue across 1 file. Review it in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
| ], | ||
| [] | ||
| ) || [], | ||
| schedule: atomSchedule.schedule.map((avail: any) => ({ |
There was a problem hiding this comment.
Using any here removes all compile-time type checking for availability items and can hide bugs. Provide the correct interface or let TypeScript infer the type instead of forcing any.
| schedule: atomSchedule.schedule.map((avail: any) => ({ | |
| schedule: atomSchedule.schedule.map((avail) => ({ |
There was a problem hiding this comment.
LGTM, please try to address this small cubic suggestion.
|
@Devanshusharma2005 done. please take a look! |
Devanshusharma2005
left a comment
Devanshusharma2005
left a comment
There was a problem hiding this comment.
LGTM, thanks for the pr.
| schedule: atomSchedule.schedule.map((avail: ScheduleQueryData["schedule"][number]) => ({ | ||
| id: avail.id ?? null, | ||
| startTime: new Date(avail.startTime), | ||
| endTime: new Date(avail.endTime), | ||
| userId: avail.userId ?? null, | ||
| eventTypeId: avail.eventTypeId ?? null, | ||
| scheduleId: avail.scheduleId ?? null, | ||
| date: avail.date ? new Date(avail.date) : null, | ||
| days: avail.days, | ||
| })), |
There was a problem hiding this comment.
I think it should be (atomSchedule.schedule || []).map
There was a problem hiding this comment.
Make sense! This fallback can save runtime issues. Fixed it. Thanks!
There was a problem hiding this comment.
I think it should be (atomSchedule.schedule || []).map
ahh coz atomSchedule.schedule might be undefined or null if no schedule is present.
apsinghdev
force-pushed
the
fix/type-error-schedule
branch
from
dc6abaf to
92af18b
Compare
Devanshusharma2005
left a comment
Devanshusharma2005
left a comment
There was a problem hiding this comment.
@apsinghdev can you please fix the tests. Marking it draft until then.
|
This PR is being marked as stale due to inactivity. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
E2E results are ready! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/platform/atoms/event-types/wrappers/EventAvailabilityTabPlatformWrapper.tsx (1)
62-62: Past bot comment resolved: no moreany.You removed the
anyin the schedule mapping and aligned withScheduleQueryData["schedule"][number]. Good resolution of the earlier bot suggestion.
🧹 Nitpick comments (4)
packages/features/eventtypes/components/tabs/availability/EventAvailabilityTab.tsx (1)
38-38: Type-only export is appropriate and low-risk.Exporting
ScheduleQueryDatawithexport typeavoids runtime coupling and makes the type available to platform wrappers. LGTM.If you anticipate broader reuse, consider relocating this alias to a small co-located
types.ts(or a domain-level types module) to reduce chances of accidental component-level import cycles in the future.packages/platform/atoms/event-types/wrappers/EventAvailabilityTabPlatformWrapper.tsx (3)
57-61: Redundant property overwrites after spreading.You spread
...atomScheduleand then immediately reassign the same properties (isManaged,readOnly,id,timeZone) to the same values. This is noise and risks accidental divergence later.Apply this diff to simplify:
- scheduleQueryData={{ - ...atomSchedule, - isManaged: atomSchedule.isManaged, - readOnly: atomSchedule.readOnly, - id: atomSchedule.id, - timeZone: atomSchedule.timeZone, + scheduleQueryData={{ + ...atomSchedule,
62-71: Strengthen typing at the result, not the callback param; prefer nullish coalescing.
- Avoid typing the
.mapcallback parameter to the target element type; it can mask mismatches between the source shape and the desired shape. Instead, make the mapping’s result satisfy the intended type.- Prefer
?? []over|| []for optional arrays to avoid treating non-null falsy values as absent (even if unlikely here).Consider this focused change:
- schedule: (atomSchedule.schedule || []).map((avail: ScheduleQueryData["schedule"][number]) => ({ + schedule: ((atomSchedule.schedule ?? []).map((avail): ScheduleQueryData["schedule"][number] => ({ id: avail.id ?? null, startTime: new Date(avail.startTime), endTime: new Date(avail.endTime), userId: avail.userId ?? null, eventTypeId: avail.eventTypeId ?? null, scheduleId: avail.scheduleId ?? null, date: avail.date ? new Date(avail.date) : null, days: avail.days, - })), + })) as ScheduleQueryData["schedule"]),Follow-up:
- If any of the nullable fields (e.g.,
id,userId) are actually non-nullable inScheduleQueryData["schedule"][number], drop the?? nulldefaults accordingly. Please run your local type-check to confirm the constraints across environments.
56-72: Optional: memoize normalization to avoid remapping on every render.Date normalization is pure but will re-run on each render causing a new array identity. If downstream components rely on referential equality for memoization, this can trigger extra renders. Memoizing on
atomSchedule.schedulekeeps renders lean.Add this above the return and use it in place of the inline mapping:
import { useMemo } from "react"; // ... const normalizedSchedule = useMemo<ScheduleQueryData["schedule"]>(() => { return ((atomSchedule.schedule ?? []).map((avail): ScheduleQueryData["schedule"][number] => ({ id: avail.id ?? null, startTime: new Date(avail.startTime), endTime: new Date(avail.endTime), userId: avail.userId ?? null, eventTypeId: avail.eventTypeId ?? null, scheduleId: avail.scheduleId ?? null, date: avail.date ? new Date(avail.date) : null, days: avail.days, })) as ScheduleQueryData["schedule"]); }, [atomSchedule.schedule]); // ... scheduleQueryData={{ ...atomSchedule, schedule: normalizedSchedule, }}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/features/eventtypes/components/tabs/availability/EventAvailabilityTab.tsx(1 hunks)packages/platform/atoms/event-types/wrappers/EventAvailabilityTabPlatformWrapper.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/eventtypes/components/tabs/availability/EventAvailabilityTab.tsxpackages/platform/atoms/event-types/wrappers/EventAvailabilityTabPlatformWrapper.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/eventtypes/components/tabs/availability/EventAvailabilityTab.tsxpackages/platform/atoms/event-types/wrappers/EventAvailabilityTabPlatformWrapper.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/eventtypes/components/tabs/availability/EventAvailabilityTab.tsxpackages/platform/atoms/event-types/wrappers/EventAvailabilityTabPlatformWrapper.tsx
🧠 Learnings (1)
📚 Learning: 2025-08-20T17:34:34.906Z
Learnt from: mdm317
PR: calcom/cal.com#23221
File: packages/features/schedules/components/NewScheduleButton.tsx:1-1
Timestamp: 2025-08-20T17:34:34.906Z
Learning: In the Cal.com codebase, server actions like `revalidateAvailabilityList()` from `app/(use-page-wrapper)/(main-nav)/availability/actions` are imported and called directly in client components within mutation `onSuccess` callbacks. This pattern is consistently used across availability-related components (both in availability-view.tsx for updates and NewScheduleButton.tsx for creates) to ensure the availability list cache is fresh after mutations.
Applied to files:
packages/platform/atoms/event-types/wrappers/EventAvailabilityTabPlatformWrapper.tsx
🧬 Code graph analysis (1)
packages/platform/atoms/event-types/wrappers/EventAvailabilityTabPlatformWrapper.tsx (1)
packages/features/eventtypes/components/tabs/availability/EventAvailabilityTab.tsx (1)
ScheduleQueryData(38-38)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
packages/platform/atoms/event-types/wrappers/EventAvailabilityTabPlatformWrapper.tsx (1)
5-5: Good: type-only import prevents runtime cycles.Using
import type { ScheduleQueryData }ensures the wrapper doesn’t pull the component module at runtime. Nice.
Ryukemeister
dismissed
Devanshusharma2005’s stale review
maybe some flaky tests, dont think theres anything here that should actually make em fail
8 participants
What does this PR do?
EventAvailabilityTabPlatformWrappercomponent #21981Visual Demo (For contributors especially)
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Pls check the
Problemssection in the terminal.Summary by cubic
Fixed a type error in the
EventAvailabilityTabPlatformWrappercomponent by updating the structure of thescheduleprop. This ensures correct typing and prevents runtime errors.