Conversation
keithwillcode
added
core
WalkthroughThe PR changes getPrefetchMonthCount to accept a new boolean parameter, prefetchNextMonth, and updates its internal decision logic: it still returns undefined when the two months are the same, returns 2 for COLUMN_VIEW when months differ, and for MONTH_VIEW will return 2 only when not in WEEK_VIEW, selecting_time, and prefetchNextMonth is false. usePrefetch.ts is updated to pass the new argument. Tests for getPrefetchMonthCount are updated to call the function with the fifth argument and include cases for prefetchNextMonth true and false across COLUMN_VIEW, WEEK_VIEW, and MONTH_VIEW. No other public/exported signatures change. Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
dosubot
bot
added
bookings
![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
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/features/bookings/Booker/components/hooks/usePrefetch.ts(1 hunks)packages/features/bookings/Booker/utils/getPrefetchMonthCount.test.ts(1 hunks)packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/Booker/utils/getPrefetchMonthCount.test.tspackages/features/bookings/Booker/components/hooks/usePrefetch.tspackages/features/bookings/Booker/utils/getPrefetchMonthCount.ts
**/*.{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/bookings/Booker/utils/getPrefetchMonthCount.test.tspackages/features/bookings/Booker/components/hooks/usePrefetch.tspackages/features/bookings/Booker/utils/getPrefetchMonthCount.ts
**/*.{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/bookings/Booker/utils/getPrefetchMonthCount.test.tspackages/features/bookings/Booker/components/hooks/usePrefetch.tspackages/features/bookings/Booker/utils/getPrefetchMonthCount.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.958Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
📚 Learning: 2025-09-18T13:38:50.958Z
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.958Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
Applied to files:
packages/features/bookings/Booker/utils/getPrefetchMonthCount.test.tspackages/features/bookings/Booker/components/hooks/usePrefetch.tspackages/features/bookings/Booker/utils/getPrefetchMonthCount.ts
🧬 Code graph analysis (1)
packages/features/bookings/Booker/utils/getPrefetchMonthCount.test.ts (2)
packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts (1)
getPrefetchMonthCount(6-32)packages/embeds/embed-core/src/types.ts (1)
BookerLayouts(6-6)
⏰ 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). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (7)
packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts (2)
21-22: LGTM!Column view correctly returns 2 regardless of the
prefetchNextMonthstate, which maintains the expected behavior for this layout.
24-29: Excellent fix for preventing duplicate API calls!The conditional logic correctly addresses the issue described in the PR:
- When
prefetchNextMonthistrue(already prefetching), the function returnsundefinedinstead of2- This prevents a second API call when
bookerStatetransitions from "loading" → "selecting_time"The detailed comment clearly explains the reasoning and conditions.
packages/features/bookings/Booker/components/hooks/usePrefetch.ts (1)
39-40: LGTM!The
prefetchNextMonthvalue is correctly computed and passed togetPrefetchMonthCount. This completes the integration of the duplicate call prevention logic.packages/features/bookings/Booker/utils/getPrefetchMonthCount.test.ts (4)
10-26: LGTM!COLUMN_VIEW tests properly updated to include the new parameter. The new test case (lines 24-26) correctly verifies that column view returns 2 even when
prefetchNextMonthis true, which aligns with the implementation.
29-43: LGTM!WEEK_VIEW tests correctly updated with the new parameter. Since week view always returns
undefined, no additional test cases are needed for differentprefetchNextMonthvalues.
46-72: Excellent test coverage for the core fix!The MONTH_VIEW tests comprehensively verify the new behavior:
- Line 47-50: Confirms returning 2 when NOT prefetching (original behavior)
- Line 52-55: Critical new test verifying undefined is returned when ALREADY prefetching - this directly tests the duplicate call prevention
The test descriptions clearly distinguish between prefetching states.
75-102: LGTM!Edge case tests properly updated to include the new parameter. The coverage remains comprehensive for invalid inputs and boundary conditions.
Ryukemeister
left a comment
Ryukemeister
left a comment
There was a problem hiding this comment.
Small NITs, looking good otherwise!
packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts
Outdated
Show resolved
Hide resolved
| // For column view, always return 2 when months are different | ||
| if (isColumnView) return 2; | ||
|
|
||
| // For month view, only add extra months when: |
There was a problem hiding this comment.
isn't this supposed to be week view and not month view? also maybe we don't need comments here as well actually
There was a problem hiding this comment.
!isWeekView was intentional to cover month view
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts (1)
23-28: Consider removing the comment block.The maintainer previously suggested that these comments may not be necessary. The variable names (
isSelectingTime,prefetchNextMonth) and the conditional logic are self-explanatory, making the five-line comment block potentially redundant.Apply this diff to remove the comment block:
- // For month view, only add extra months when: - // 1. User is selecting time AND - // 2. We're NOT already prefetching the next month - // This prevents duplicate calls when bookerState changes to "selecting_time" - // after the initial data load if (!isWeekView && isSelectingTime && !prefetchNextMonth) return 2;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts
**/*.{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/bookings/Booker/utils/getPrefetchMonthCount.ts
**/*.{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/bookings/Booker/utils/getPrefetchMonthCount.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.958Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
📚 Learning: 2025-09-18T13:38:50.958Z
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.958Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
Applied to files:
packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts
🔇 Additional comments (1)
packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts (1)
10-11: LGTM! Parameter addition correctly addresses duplicate call issue.The new
prefetchNextMonthparameter enables the function to determine whether a prefetch is already in progress, preventing the duplicate API calls described in the PR objectives.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts (1)
21-32: Consider simplifying the comments.The inline comments are quite verbose. Past reviewers suggested that they may not be necessary, and the code logic is relatively self-explanatory with well-named variables.
Consider reducing the comments to a single brief line or removing them entirely:
- // Column view always needs 2 months because it displays multiple weeks side-by-side, - // regardless of user state or whether we're already prefetching if (isColumnView) return 2; - // Month view conditionally needs an extra month for performance optimization. - // Only return 2 when: - // 1. User is selecting time slots (improves UX by preloading more availability) - // 2. We're NOT already prefetching next month (prevents duplicate API calls) - // - // If prefetchNextMonth is already true (e.g., viewing dates after 15th of month), - // we don't need extra months since the next month is already being fetched via prefetchNextMonth if (!isWeekView && isSelectingTime && !prefetchNextMonth) return 2;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts
**/*.{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/bookings/Booker/utils/getPrefetchMonthCount.ts
**/*.{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/bookings/Booker/utils/getPrefetchMonthCount.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.958Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
📚 Learning: 2025-09-18T13:38:50.958Z
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.958Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
Applied to files:
packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts
🔇 Additional comments (1)
packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts (1)
6-12: Logic correctly addresses the duplicate API call issue.The new
prefetchNextMonthparameter effectively prevents duplicate API calls by ensuring that when the next month is already being prefetched, the function doesn't request an additional prefetch. The condition on line 32 (!prefetchNextMonth) ensures that the month count only increases when a prefetch is not already in progress.
E2E results are ready! |
4 participants
What does this PR do?
When the initial getSchedule request completed, bookerState automatically transitioned from "loading" → "selecting_time". This caused getPrefetchMonthCount to change from undefined → 2, triggering a second API call with different endTime (e.g., Nov 30 → Dec 31).
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?