refactor: move WebWrapper files from packages/platform to apps/web/modules#9
Conversation
📝 WalkthroughWalkthroughThis pull request performs a major module reorganization, moving web wrapper components from the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Author
|
@coderabbitai review |
1 similar comment
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
🤖 Fix all issues with AI agents
In `@apps/web/components/apps/DestinationCalendarSettingsWebWrapper.tsx`:
- Around line 3-4: The imports use deep relative paths across package
boundaries; replace them with public package imports (e.g., import {
AtomsWrapper, DestinationCalendarSettings } from "@calcom/atoms") or, if
AtomsWrapper is internal-only, export AtomsWrapper from the package root (the
`@calcom/atoms` index) so the component can be imported via the package rather
than "../../../../packages/..."; update the import statements in
DestinationCalendarSettingsWebWrapper.tsx to reference the package public API
and ensure the package's index.ts re-exports AtomsWrapper and
DestinationCalendarSettings if missing.
In `@apps/web/modules/bookings/components/BookerWebWrapper.tsx`:
- Around line 4-5: The component uses router but no longer imports or
initializes it; restore the Next.js useRouter hook by importing useRouter and
calling it inside BookerWebWrapperComponent to produce the router instance used
by onOverlaySwitchStateChange, the callback that calls router.push(), and the
other places that reference router (the push calls at newPath,
"/apps/categories/calendar", and newUrl.toString()); ensure you replace any
lingering references to a global router with the local const router returned
from useRouter and update dependency arrays (e.g., for the useCallback/useEffect
hooks) to include router if needed.
🧹 Nitpick comments (3)
packages/lib/bookings/hostGroupUtils.ts (1)
41-60: Utility function extracted and consolidated - consider adding documentation.The
sortHostsfunction logic is correct and handles nullable fields with appropriate defaults. Since this is now a shared utility used by multiple components, consider adding a brief JSDoc comment explaining the sorting semantics:📝 Suggested documentation
+/** + * Sorts hosts for round-robin assignment. + * - When isRRWeightsEnabled: sorts by weight descending (higher weight first), + * then by priority ascending (lower priority number first) when weights are equal. + * - When !isRRWeightsEnabled: sorts by priority descending (higher priority first). + * `@param` hostA - First host with nullable priority and weight + * `@param` hostB - Second host with nullable priority and weight + * `@param` isRRWeightsEnabled - Whether round-robin weights feature is enabled + */ export function sortHosts( hostA: { priority: number | null; weight: number | null }, hostB: { priority: number | null; weight: number | null }, isRRWeightsEnabled: boolean ) {apps/web/modules/event-types/components/EventTypeWebWrapper.tsx (2)
52-91: Redundant.then((mod) => mod)in dynamic imports.Several dynamic imports use
.then((mod) => mod)which is a no-op:// Lines 53, 58, 64, 68, 76 - redundant pattern import("./tabs/setup/EventSetupTabWebWrapper").then((mod) => mod) // Lines 72, 80, 86, 90 - correct pattern for named exports import("./tabs/instant/EventInstantTab").then((mod) => mod.EventInstantTab)The first pattern returns the module unchanged. Consider removing the redundant
.then()calls for cleaner code, similar to lines 61 and 83.♻️ Suggested cleanup
const EventSetupTab = dynamic( - () => import("./tabs/setup/EventSetupTabWebWrapper").then((mod) => mod), + () => import("./tabs/setup/EventSetupTabWebWrapper"), { loading: () => null } ); -const EventAvailabilityTab = dynamic(() => - import("./tabs/availability/EventAvailabilityTabWebWrapper").then((mod) => mod) -); +const EventAvailabilityTab = dynamic( + () => import("./tabs/availability/EventAvailabilityTabWebWrapper") +); const EventLimitsTab = dynamic(() => - import("./tabs/limits/EventLimitsTabWebWrapper").then((mod) => mod) + import("./tabs/limits/EventLimitsTabWebWrapper") ); const EventAdvancedTab = dynamic(() => - import("./tabs/advanced/EventAdvancedWebWrapper").then((mod) => mod) + import("./tabs/advanced/EventAdvancedWebWrapper") ); const EventRecurringTab = dynamic(() => - import("./tabs/recurring/EventRecurringWebWrapper").then((mod) => mod) + import("./tabs/recurring/EventRecurringWebWrapper") );
236-250: Inconsistent JSX indentation.The indentation for
EventSetupTabandEventAvailabilityTabdiffers from other components in the sametabMapobject (e.g.,EventTeamAssignmentTabon lines 253-258,EventAdvancedTabon lines 262-269). Consider aligning the formatting for consistency.♻️ Suggested formatting alignment
const tabMap = { setup: ( - <EventSetupTab - eventType={eventType} - locationOptions={locationOptions} - team={team} - teamMembers={teamMembers} - destinationCalendar={destinationCalendar} - /> + <EventSetupTab + eventType={eventType} + locationOptions={locationOptions} + team={team} + teamMembers={teamMembers} + destinationCalendar={destinationCalendar} + /> ), availability: ( - <EventAvailabilityTab - eventType={eventType} - isTeamEvent={!!team} - user={user} - teamMembers={teamMembers} - /> + <EventAvailabilityTab + eventType={eventType} + isTeamEvent={!!team} + user={user} + teamMembers={teamMembers} + /> ),
Comment on lines
+3
to
+4
| import { AtomsWrapper } from "../../../../packages/platform/atoms/src/components/atoms-wrapper"; | ||
| import { DestinationCalendarSettings } from "../../../../packages/platform/atoms/destination-calendar/DestinationCalendar"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check the file in question
echo "=== Current imports in DestinationCalendarSettingsWebWrapper.tsx ==="
head -10 apps/web/components/apps/DestinationCalendarSettingsWebWrapper.tsx
# Check if packages/platform/atoms/package.json exists and show exports
echo ""
echo "=== Checking atoms package.json ==="
if [ -f packages/platform/atoms/package.json ]; then
cat packages/platform/atoms/package.json | jq '.exports // "No exports field"' 2>/dev/null || cat packages/platform/atoms/package.json | grep -A 10 '"exports"' 2>/dev/null || echo "Could not parse exports"
else
echo "packages/platform/atoms/package.json not found"
fi
# Look for workspace/alias configurations
echo ""
echo "=== Checking for alias configurations ==="
if [ -f tsconfig.json ]; then
echo "Found tsconfig.json:"
cat tsconfig.json | jq '.compilerOptions.paths // {}' 2>/dev/null || echo "No paths found"
fi
# Check monorepo root package.json workspaces
echo ""
echo "=== Checking monorepo structure ==="
cat package.json | jq '.workspaces // "No workspaces"' 2>/dev/null || echo "Could not parse"Repository: agentic-review-benchmarks/cal.com-combined-coderabbit
Length of output: 2602
🏁 Script executed:
#!/bin/bash
# Search for other imports from `@calcom/atoms` or similar
echo "=== Searching for `@calcom/atoms` imports in codebase ==="
rg '@calcom/atoms' --type ts --type tsx -m 10 | head -20
# Also check for relative imports from packages/platform
echo ""
echo "=== Checking relative imports from packages/platform ==="
rg '\.\./\.\./\.\./\.\./packages/platform' --type ts --type tsx -m 10Repository: agentic-review-benchmarks/cal.com-combined-coderabbit
Length of output: 283
🏁 Script executed:
#!/bin/bash
# Search for `@calcom/atoms` or similar aliases without type flags
echo "=== Searching for `@calcom/atoms` usage ==="
rg '@calcom/atoms' -m 10 || echo "No matches found"
# Check tsconfig files for path aliases
echo ""
echo "=== Checking all tsconfig files for path aliases ==="
find . -name "tsconfig.json" -type f ! -path "*/node_modules/*" ! -path "*/.git/*" -exec grep -l "paths" {} \; | head -5
# Look for `@calcom` alias definitions
echo ""
echo "=== Searching for `@calcom` path alias definitions ==="
rg '@calcom' -A 2 -B 2 | grep -E '"@calcom|baseUrl|paths' | head -20
# Check the actual atoms package directory structure
echo ""
echo "=== Checking atoms package directory structure ==="
if [ -d packages/platform/atoms ]; then
echo "Contents of packages/platform/atoms:"
find packages/platform/atoms -maxdepth 2 -type f -name "*.tsx" -o -name "*.ts" | grep -E "(atoms-wrapper|DestinationCalendar)" | head -10
fiRepository: agentic-review-benchmarks/cal.com-combined-coderabbit
Length of output: 20273
🏁 Script executed:
#!/bin/bash
# Look for how the file currently resolves its imports
echo "=== Checking if atoms-wrapper or DestinationCalendar files exist ==="
find packages/platform/atoms -name "*atoms-wrapper*" -o -name "*DestinationCalendar*" 2>/dev/null
# Check what the review comment imports actually point to
echo ""
echo "=== Checking if those specific paths exist ==="
if [ -f "packages/platform/atoms/src/components/atoms-wrapper.tsx" ]; then
echo "Found: packages/platform/atoms/src/components/atoms-wrapper.tsx"
elif [ -f "packages/platform/atoms/src/components/atoms-wrapper.ts" ]; then
echo "Found: packages/platform/atoms/src/components/atoms-wrapper.ts"
else
echo "Not found: packages/platform/atoms/src/components/atoms-wrapper"
fi
if [ -f "packages/platform/atoms/destination-calendar/DestinationCalendar.tsx" ]; then
echo "Found: packages/platform/atoms/destination-calendar/DestinationCalendar.tsx"
else
echo "Not found: packages/platform/atoms/destination-calendar/DestinationCalendar.tsx"
fi
# Check root tsconfig
echo ""
echo "=== Root tsconfig.json paths configuration ==="
cat tsconfig.json 2>/dev/null | head -50Repository: agentic-review-benchmarks/cal.com-combined-coderabbit
Length of output: 778
🏁 Script executed:
#!/bin/bash
# Check what exports are actually available for destination-calendar in package.json
echo "=== Checking atoms package.json for destination-calendar exports ==="
cat packages/platform/atoms/package.json | jq '.exports | keys[] | select(contains("destination"))'
# Check how DestinationCalendarSettings is currently exported/imported
echo ""
echo "=== Looking for DestinationCalendarSettings imports ==="
rg 'DestinationCalendarSettings' -A 1 -B 1 | head -30
# Check the index file of atoms to see what's exported
echo ""
echo "=== Checking atoms main index files ==="
if [ -f packages/platform/atoms/index.ts ]; then
echo "Found index.ts:"
cat packages/platform/atoms/index.ts | head -50
fi
# Check if there's a root export file
echo ""
echo "=== Checking for main exports file ==="
find packages/platform/atoms -maxdepth 1 -name "*.ts" -o -name "*.tsx" | head -5Repository: agentic-review-benchmarks/cal.com-combined-coderabbit
Length of output: 6762
🏁 Script executed:
#!/bin/bash
# Look at the DestinationCalendarSettingsWebWrapper to understand current state
echo "=== Checking the actual file imports again ==="
head -20 apps/web/components/apps/DestinationCalendarSettingsWebWrapper.tsx
# Check SelectedCalendarsSettingsWebWrapper to see if it's a parallel file
echo ""
echo "=== Checking parallel SelectedCalendarsSettingsWebWrapper ==="
head -20 apps/web/modules/calendars/components/SelectedCalendarsSettingsWebWrapper.tsx
# Search for how DestinationCalendarSettings is properly imported
echo ""
echo "=== Searching for existing DestinationCalendarSettings imports ==="
rg 'import.*DestinationCalendarSettings' -A 0 -B 0Repository: agentic-review-benchmarks/cal.com-combined-coderabbit
Length of output: 2742
🏁 Script executed:
#!/bin/bash
# Check the destination-calendar index file to see what it exports
echo "=== Checking destination-calendar/index.ts ==="
if [ -f packages/platform/atoms/destination-calendar/index.ts ]; then
cat packages/platform/atoms/destination-calendar/index.ts
fi
# Check if AtomsWrapper is exported anywhere
echo ""
echo "=== Searching for AtomsWrapper exports ==="
rg 'export.*AtomsWrapper' packages/platform/atoms/
# Check the atoms-wrapper file itself
echo ""
echo "=== Checking atoms-wrapper.tsx ==="
head -30 packages/platform/atoms/src/components/atoms-wrapper.tsx
# Check how it's used in the platform examples
echo ""
echo "=== Checking atoms-wrapper usage in examples ==="
rg 'AtomsWrapper' packages/platform/examples/ -A 1 -B 1 | head -20Repository: agentic-review-benchmarks/cal.com-combined-coderabbit
Length of output: 1077
🏁 Script executed:
#!/bin/bash
# Check what the destination-calendar exports actually expose
echo "=== Full destination-calendar index content ==="
cat packages/platform/atoms/destination-calendar/index.ts 2>/dev/null || echo "File not found"
# Check package.json exports for specific sub-paths
echo ""
echo "=== Checking package.json for sub-path exports ==="
cat packages/platform/atoms/package.json | jq '.exports | to_entries[] | select(.value | tostring | contains("wrapper") or contains("AtomsWrapper"))'
# Check if AtomsWrapper is in any index file
echo ""
echo "=== Searching all index files for AtomsWrapper ==="
find packages/platform/atoms -name "index.ts" -exec grep -l "AtomsWrapper" {} \;Repository: agentic-review-benchmarks/cal.com-combined-coderabbit
Length of output: 364
🏁 Script executed:
#!/bin/bash
# Check what the DestinationCalendarSettings actually exports from DestinationCalendar file
echo "=== Checking what DestinationCalendar.tsx exports ==="
head -50 packages/platform/atoms/destination-calendar/DestinationCalendar.tsx | grep -E "export|import"
# Check the full package.json exports field again to see all available paths
echo ""
echo "=== Complete exports field from package.json ==="
cat packages/platform/atoms/package.json | jq -r '.exports | to_entries | .[] | "\(.key): \(.value)"' | sortRepository: agentic-review-benchmarks/cal.com-combined-coderabbit
Length of output: 1985
🏁 Script executed:
#!/bin/bash
# Verify what's actually exported from destination-calendar index
echo "=== Checking DestinationCalendarSettingsPlatformWrapper ==="
head -20 packages/platform/atoms/destination-calendar/wrappers/DestinationCalendarSettingsPlatformWrapper.tsx | grep -E "export|import"
# Check if DestinationCalendarSettings export exists anywhere
echo ""
echo "=== Searching for 'export.*DestinationCalendarSettings' (not PlatformWrapper) ==="
rg 'export.*DestinationCalendarSettings[^P]' packages/platform/atoms/Repository: agentic-review-benchmarks/cal.com-combined-coderabbit
Length of output: 959
Avoid deep relative imports crossing package boundaries.
Using relative paths like ../../../../packages/platform/atoms/... to import from another package in a monorepo is fragile and breaks conventional monorepo practices. However, the suggested import paths need correction.
The package exports don't include sub-path exports for destination-calendar/DestinationCalendar or atoms-wrapper. The correct approach is:
-import { AtomsWrapper } from "../../../../packages/platform/atoms/src/components/atoms-wrapper";
-import { DestinationCalendarSettings } from "../../../../packages/platform/atoms/destination-calendar/DestinationCalendar";
+import { AtomsWrapper } from "@calcom/atoms/src/components/atoms-wrapper";
+import { DestinationCalendarSettings } from "@calcom/atoms";Alternatively, if the intent is to use only the internal wrapper component, AtomsWrapper should be exported from the main @calcom/atoms index to make it a public API.
🤖 Prompt for AI Agents
In `@apps/web/components/apps/DestinationCalendarSettingsWebWrapper.tsx` around
lines 3 - 4, The imports use deep relative paths across package boundaries;
replace them with public package imports (e.g., import { AtomsWrapper,
DestinationCalendarSettings } from "@calcom/atoms") or, if AtomsWrapper is
internal-only, export AtomsWrapper from the package root (the `@calcom/atoms`
index) so the component can be imported via the package rather than
"../../../../packages/..."; update the import statements in
DestinationCalendarSettingsWebWrapper.tsx to reference the package public API
and ensure the package's index.ts re-exports AtomsWrapper and
DestinationCalendarSettings if missing.
Comment on lines
+4
to
+5
| import { useSearchParams, usePathname } from "next/navigation"; | ||
| import { useMemo, useCallback, useEffect } from "react"; |
There was a problem hiding this comment.
Critical: router is used but never declared.
The useRouter hook was removed from imports, but router is still referenced multiple times in the component:
- Line 193:
router.push()inonOverlaySwitchStateChange - Line 195:
routerin dependency array - Line 228:
router.push(newPath) - Line 232:
router.push("/apps/categories/calendar") - Line 238:
router.push(newUrl.toString())
This will cause a ReferenceError at runtime when any of these callbacks are invoked.
🐛 Proposed fix: Restore useRouter import and declaration
-import { useSearchParams, usePathname } from "next/navigation";
+import { useSearchParams, usePathname, useRouter } from "next/navigation";Then add inside BookerWebWrapperComponent:
const BookerWebWrapperComponent = (props: BookerWebWrapperAtomProps) => {
+ const router = useRouter();
const pathname = usePathname();🤖 Prompt for AI Agents
In `@apps/web/modules/bookings/components/BookerWebWrapper.tsx` around lines 4 -
5, The component uses router but no longer imports or initializes it; restore
the Next.js useRouter hook by importing useRouter and calling it inside
BookerWebWrapperComponent to produce the router instance used by
onOverlaySwitchStateChange, the callback that calls router.push(), and the other
places that reference router (the push calls at newPath,
"/apps/categories/calendar", and newUrl.toString()); ensure you replace any
lingering references to a global router with the local const router returned
from useRouter and update dependency arrays (e.g., for the useCallback/useEffect
hooks) to include router if needed.
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
2 participants
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 agentic-review-benchmarks#9
Summary by CodeRabbit