-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: move WebWrapper files from packages/platform to apps/web/modules #36
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_full_base_refactor_move_webwrapper_files_from_packages_platform_to_apps_web_modules_pr9
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request migrates WebWrapper components from the atoms package to web modules and consolidates the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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
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
🤖 Fix all issues with AI agents
In `@apps/web/components/apps/DestinationCalendarSettingsWebWrapper.tsx`:
- Around line 3-4: Current imports bypass the package entry by importing
AtomsWrapper and DestinationCalendarSettings from deep source paths; update
imports to use the package entry and transpilation settings by replacing deep
imports with the public exports from packages/platform/atoms (use
DestinationCalendarSettingsPlatformWrapper for DestinationCalendarSettings and
the exported AtomsWrapper if added to the package entry), and add
`@calcom/platform-atoms` to the Next.js transpilePackages array in next.config.ts
(or alternatively add a tsconfig path alias pointing to the package entry) so
the module is transpiled in production.
In `@apps/web/modules/bookings/components/BookerWebWrapper.tsx`:
- Around line 4-6: The component BookerWebWrapper uses router but removed the
useRouter hook; re-add the import "useRouter" from "next/navigation" and declare
a router instance inside the component (e.g., const router = useRouter())
alongside other hooks (useSearchParams, usePathname, useMemo, etc.) so the
existing references to router (used in callbacks and effects) resolve and follow
React hook rules.
🧹 Nitpick comments (2)
apps/web/modules/event-types/components/tabs/availability/EventAvailabilityTab.tsx (1)
8-8: Potential circular import pattern.
EventAvailabilityTab.tsximports a type fromEventAvailabilityTabWebWrapper.tsx, while the wrapper file importsEventAvailabilityTabfrom this file. While TypeScript handles circular type imports, this pattern can cause issues if not carefully managed. Consider extracting shared types to a separate types file.📁 Suggested types extraction
Create a shared types file like
EventAvailabilityTab.types.ts:// EventAvailabilityTab.types.ts import type { trpc } from "@calcom/trpc/react"; export type GetAllSchedulesByUserIdQueryType = typeof trpc.viewer.availability.schedule.getAllSchedulesByUserId.useQuery; // Move other shared types here...Then both files can import from this central location.
apps/web/components/apps/CalendarListContainer.tsx (1)
6-7: Import path style inconsistency.Line 6 uses a relative path (
./DestinationCalendarSettingsWebWrapper) while Line 7 and all other imports in the file use the absolute alias pattern (@calcom/web/modules/calendars/components/SelectedCalendarsSettingsWebWrapper). Align Line 6 with the project's import convention by using@calcom/web/components/apps/DestinationCalendarSettingsWebWrapper.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
apps/web/app/(use-page-wrapper)/event-types/[type]/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/my-account/conferencing/loading.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/my-account/conferencing/page.tsxapps/web/components/apps/CalendarListContainer.tsxapps/web/components/apps/DestinationCalendarSettingsWebWrapper.tsxapps/web/modules/apps/components/ConferencingAppsViewWebWrapper.tsxapps/web/modules/bookings/components/BookerWebWrapper.tsxapps/web/modules/calendars/components/SelectedCalendarsSettingsWebWrapper.tsxapps/web/modules/d/[link]/d-type-view.tsxapps/web/modules/event-types/components/AddMembersWithSwitch.tsxapps/web/modules/event-types/components/AddMembersWithSwitchWebWrapper.tsxapps/web/modules/event-types/components/EventTypeWebWrapper.tsxapps/web/modules/event-types/components/HostEditDialogs.tsxapps/web/modules/event-types/components/tabs/advanced/EventAdvancedTab.tsxapps/web/modules/event-types/components/tabs/advanced/EventAdvancedWebWrapper.tsxapps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTab.tsxapps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTabWebWrapper.tsxapps/web/modules/event-types/components/tabs/availability/EventAvailabilityTab.tsxapps/web/modules/event-types/components/tabs/availability/EventAvailabilityTabWebWrapper.tsxapps/web/modules/event-types/components/tabs/limits/EventLimitsTabWebWrapper.tsxapps/web/modules/event-types/components/tabs/recurring/EventRecurringWebWrapper.tsxapps/web/modules/event-types/components/tabs/setup/EventSetupTabWebWrapper.tsxapps/web/modules/org/[orgSlug]/instant-meeting/team/[slug]/[type]/instant-meeting-view.tsxapps/web/modules/team/type-view-cached.tsxapps/web/modules/team/type-view.tsxapps/web/modules/users/views/users-type-public-view.tsxpackages/lib/bookings/hostGroupUtils.tspackages/platform/atoms/booker/index.tspackages/platform/atoms/event-types/hooks/useEventTypeForm.tspackages/platform/atoms/package.json
💤 Files with no reviewable changes (1)
- packages/platform/atoms/booker/index.ts
🔇 Additional comments (28)
apps/web/modules/event-types/components/tabs/recurring/EventRecurringWebWrapper.tsx (1)
8-8: Looks good.Default export aligns with the wrapper component and keeps the module surface consistent.
apps/web/modules/event-types/components/AddMembersWithSwitch.tsx (1)
7-7: LGTM!The import path change from
@calcom/atoms/add-members-switch/AddMembersWithSwitchWebWrapperto the local relative path./AddMembersWithSwitchWebWrapperis consistent with the PR objective of moving WebWrapper files to local modules.apps/web/modules/event-types/components/AddMembersWithSwitchWebWrapper.tsx (1)
3-5: LGTM!The import path changes from
@calcom/web/modules/event-types/components/AddMembersWithSwitchto the local relative path./AddMembersWithSwitchare consistent with the PR objective of consolidating module imports locally.apps/web/app/(use-page-wrapper)/settings/(settings-layout)/my-account/conferencing/loading.tsx (1)
1-1: Import path correctly updated for the module migration.The
SkeletonLoaderis properly exported from@calcom/web/modules/apps/components/ConferencingAppsViewWebWrapperand the import statement is valid. The component logic remains unchanged.packages/platform/atoms/event-types/hooks/useEventTypeForm.ts (1)
11-11: Import update looks good. CentralizingsortHostsin the shared utility is a clean consolidation.apps/web/modules/event-types/components/tabs/setup/EventSetupTabWebWrapper.tsx (1)
6-8: LGTM for the local import swap. No behavior change implied here.apps/web/modules/apps/components/ConferencingAppsViewWebWrapper.tsx (1)
14-16: Local component imports look fine.apps/web/app/(use-page-wrapper)/settings/(settings-layout)/my-account/conferencing/page.tsx (1)
4-4: Import path update looks good.apps/web/modules/event-types/components/tabs/limits/EventLimitsTabWebWrapper.tsx (1)
8-8: LGTM.apps/web/modules/bookings/components/BookerWebWrapper.tsx (1)
35-35: Local Booker import looks good.apps/web/modules/d/[link]/d-type-view.tsx (1)
3-3: LGTM: import path migration is consistent.
Line 3 cleanly switches BookerWebWrapper to the web/modules location.apps/web/modules/org/[orgSlug]/instant-meeting/team/[slug]/[type]/instant-meeting-view.tsx (1)
5-5: LGTM: import path migration is consistent.
Line 5 update aligns with the new web/modules path.apps/web/modules/team/type-view.tsx (1)
6-6: LGTM: import path migration is consistent.
Line 6 change matches the repo-wide BookerWebWrapper move.apps/web/modules/users/views/users-type-public-view.tsx (1)
6-6: LGTM: import path migration is consistent.
Line 6 is a clean switch to the web/modules path.apps/web/modules/team/type-view-cached.tsx (1)
3-3: LGTM: import path migration is consistent.
Line 3 update aligns the wrapper and type with the new web/modules location.packages/lib/bookings/hostGroupUtils.ts (1)
41-60: LGTM! Clean centralization of the sortHosts utility.The sorting logic is clear:
- When
isRRWeightsEnabled: Higher weight takes precedence; equal weights fall back to lower priority number winning- When disabled: Higher priority number wins
The default values (weight: 100, priority: 2) provide sensible fallbacks for null values.
apps/web/modules/event-types/components/HostEditDialogs.tsx (1)
13-13: LGTM! Import consolidation is correct.The
sortHostsfunction is now imported from the shared utility module alongside the related host grouping functions, improving code organization and eliminating duplication.apps/web/modules/calendars/components/SelectedCalendarsSettingsWebWrapper.tsx (1)
15-15: LGTM! Import path updated to use the new atoms export.The import now uses the explicit
@calcom/atoms/selected-calendars/SelectedCalendarsSettingspath, which aligns with the newly added export in the atoms package.apps/web/modules/event-types/components/tabs/advanced/EventAdvancedTab.tsx (1)
9-13: LGTM! Import path correctly updated to web modules.The
SelectedCalendarsSettingsWebWrapperand related exports are now imported from the web modules path (@calcom/web/modules/calendars/components/), consistent with the PR's goal of relocating WebWrapper components.apps/web/modules/event-types/components/tabs/advanced/EventAdvancedWebWrapper.tsx (1)
3-5: LGTM! Appropriate use of relative imports for co-located files.Using relative imports (
./EventAdvancedTab) for files in the same directory is cleaner and reduces dependency on alias configurations.packages/platform/atoms/package.json (1)
59-74: No issues found. The WebWrapper exports mentioned in the original comment were never part of the@calcom/atomspackage exports. All WebWrapper components exist in@calcom/webmodules and are imported from there. The atoms package exports only the PlatformWrapper component (AddMembersWithSwitchPlatformWrapper) and various hooks and components, none of which were removed in this change.Likely an incorrect or invalid review comment.
apps/web/app/(use-page-wrapper)/event-types/[type]/page.tsx (1)
9-9: LGTM!The import path update for
EventTypeWebWrapperaligns with the PR objective to consolidate WebWrapper files inapps/web/modules. The component usage remains unchanged.apps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTabWebWrapper.tsx (1)
1-4: LGTM!The import change to a relative path (
./EventTeamAssignmentTab) is appropriate for co-located files in the same directory. The wrapper logic remains unchanged.apps/web/modules/event-types/components/tabs/availability/EventAvailabilityTabWebWrapper.tsx (1)
8-10: LGTM!The import path updates use appropriate relative paths for co-located modules. The
TeamMemberstype import navigates correctly to the parentEventTypemodule.apps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTab.tsx (1)
17-17: LGTM! MovingsortHoststo@calcom/lib/bookings/hostGroupUtilsis a good refactoring choice. Centralizing utility functions in a shared library improves maintainability and reduces duplication.apps/web/modules/event-types/components/EventTypeWebWrapper.tsx (3)
24-31: Imports aligned with the new module layout.
Looks consistent with the refactor targets.
236-251: Formatting-only change in tabMap.
No functional concerns here.
48-90: All dynamic imports are correctly configured and will resolve properly at runtime.All modules export either a default export or a named export that matches the import pattern:
- Modules with default exports use bare
import()or.then((mod) => mod)- Modules with named exports use
.then((mod) => mod.NamedExport)correctlyNo changes needed.
Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| 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.
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
# Verify Next.js transpilation settings and path aliases relevant to packages/platform/atoms.
echo "== next.config files =="
fd -a 'next.config' -E node_modules -E .next
for f in $(fd -a 'next.config' -E node_modules -E .next); do
echo "---- $f ----"
rg -n "transpilePackages|externalDir|outputFileTracingRoot" "$f" || true
done
echo "== tsconfig path aliases =="
for f in $(fd -a 'tsconfig*.json' -E node_modules -E .next); do
echo "---- $f ----"
rg -n "\"@calcom/|packages/platform/atoms\"" "$f" || true
doneRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 7713
🏁 Script executed:
#!/bin/bash
# Check the package.json and entry point of packages/platform/atoms
echo "== Looking for packages/platform/atoms package.json =="
find . -path "*/packages/platform/atoms/package.json" -type f 2>/dev/null | head -5
echo "== Inspect atoms package structure =="
find packages/platform/atoms -maxdepth 3 -type f -name "package.json" -o -name "index.ts" -o -name "index.tsx" 2>/dev/null | head -20Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 988
🏁 Script executed:
#!/bin/bash
# Check other imports from packages/platform/atoms in the codebase
echo "== Other imports from packages/platform/atoms =="
rg "from ['\"].*packages/platform/atoms" apps/web --type ts --type tsx -n | head -20Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 179
🏁 Script executed:
#!/bin/bash
# Get the actual transpilePackages array from apps/web/next.config.ts
echo "== apps/web/next.config.ts transpilePackages section =="
sed -n '230,260p' apps/web/next.config.tsRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 884
🏁 Script executed:
#!/bin/bash
# Check what's exported from packages/platform/atoms destination-calendar entry point
echo "== packages/platform/atoms/destination-calendar/index.ts =="
cat packages/platform/atoms/destination-calendar/index.ts
echo ""
echo "== packages/platform/atoms/src/components structure =="
find packages/platform/atoms/src/components -type f -name "*.ts" -o -name "*.tsx" 2>/dev/null | head -10Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 800
🏁 Script executed:
#!/bin/bash
# Check if atoms-wrapper is exported from the destination-calendar package or main atoms
echo "== Search for atoms-wrapper in packages/platform/atoms =="
find packages/platform/atoms -name "*atoms-wrapper*" -o -name "*AtomWrapper*" 2>/dev/null
echo ""
echo "== Check main atoms index.ts for atoms-wrapper export =="
rg -n "atoms-wrapper|AtomsWrapper" packages/platform/atoms/index.tsRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 258
🏁 Script executed:
#!/bin/bash
# Search for all deep imports from packages/platform/atoms across the whole codebase
echo "== All deep imports from packages/platform/atoms (across entire repo) =="
rg "from ['\"].*packages/platform/atoms/(src/|[a-z-]+/)" --type ts -A 0 | head -30Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 1508
🏁 Script executed:
#!/bin/bash
# Check if DestinationCalendar file exists
echo "== packages/platform/atoms/destination-calendar/ directory contents =="
ls -la packages/platform/atoms/destination-calendar/
echo ""
echo "== Check for DestinationCalendarSettings export =="
rg -n "DestinationCalendarSettings" packages/platform/atoms/destination-calendar/ || echo "Not found in destination-calendar"Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 1705
🏁 Script executed:
#!/bin/bash
# Check the main atoms index.ts to see what's exported
echo "== packages/platform/atoms/index.ts (main entry point) =="
cat packages/platform/atoms/index.ts | head -50Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 3731
🏁 Script executed:
#!/bin/bash
# Check the atoms-wrapper export
echo "== packages/platform/atoms/src/components/atoms-wrapper.tsx (first 20 lines) =="
head -20 packages/platform/atoms/src/components/atoms-wrapper.tsxRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 746
Add @calcom/platform-atoms to transpilePackages and prefer entry-point exports.
Lines 3–4 import directly from package source paths (atoms-wrapper and DestinationCalendar file), bypassing the main entry point. The AtomsWrapper is not exported from the package entry, but DestinationCalendarSettings is available as DestinationCalendarSettingsPlatformWrapper from packages/platform/atoms. Additionally, @calcom/platform-atoms is not listed in Next.js transpilePackages at apps/web/next.config.ts (which currently includes @calcom/app-store, @calcom/features, @calcom/lib, etc.), which may cause transpilation issues in production. Either add @calcom/platform-atoms to transpilePackages or consider creating a tsconfig path alias to normalize these imports.
🤖 Prompt for AI Agents
In `@apps/web/components/apps/DestinationCalendarSettingsWebWrapper.tsx` around
lines 3 - 4, Current imports bypass the package entry by importing AtomsWrapper
and DestinationCalendarSettings from deep source paths; update imports to use
the package entry and transpilation settings by replacing deep imports with the
public exports from packages/platform/atoms (use
DestinationCalendarSettingsPlatformWrapper for DestinationCalendarSettings and
the exported AtomsWrapper if added to the package entry), and add
`@calcom/platform-atoms` to the Next.js transpilePackages array in next.config.ts
(or alternatively add a tsconfig path alias pointing to the package entry) so
the module is transpiled in production.
| import { useSearchParams, usePathname } from "next/navigation"; | ||
| import { useMemo, useCallback, useEffect } from "react"; | ||
| import React from "react"; |
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:
cat -n apps/web/modules/bookings/components/BookerWebWrapper.tsx | head -50Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 2890
🏁 Script executed:
cat -n apps/web/modules/bookings/components/BookerWebWrapper.tsx | sed -n '190,240p'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 2125
🏁 Script executed:
cat -n apps/web/modules/bookings/components/BookerWebWrapper.tsx | sed -n '40,100p'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 2927
🏁 Script executed:
rg "const router\s*=" apps/web/modules/bookings/components/BookerWebWrapper.tsxRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 68
🏁 Script executed:
rg "router" apps/web/modules/bookings/components/BookerWebWrapper.tsx -B 2 -A 2Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 1414
🏁 Script executed:
cat -n apps/web/modules/bookings/components/BookerWebWrapper.tsx | sed -n '100,180p'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 3280
🏁 Script executed:
cat -n apps/web/modules/bookings/components/BookerWebWrapper.tsx | sed -n '180,210p'Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 1308
🏁 Script executed:
wc -l apps/web/modules/bookings/components/BookerWebWrapper.tsxRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 147
🏁 Script executed:
grep -n "router" apps/web/modules/bookings/components/BookerWebWrapper.tsxRepository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 454
🏁 Script executed:
cat -n apps/web/modules/bookings/components/BookerWebWrapper.tsx | grep -E "(function|const.*=.*\(|router\s*:|router\s*=)" | head -30Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 1946
router is undefined after useRouter import was removed. The variable is used at lines 193, 228, 232, and 238, but never declared. Re-introduce the useRouter import and declare the router instance.
🛠️ Proposed fix
-import { useSearchParams, usePathname } from "next/navigation";
+import { useSearchParams, usePathname, useRouter } from "next/navigation"; const BookerWebWrapperComponent = (props: BookerWebWrapperAtomProps) => {
const pathname = usePathname();
const searchParams = useSearchParams();
+ const router = useRouter();📝 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.
| import { useSearchParams, usePathname } from "next/navigation"; | |
| import { useMemo, useCallback, useEffect } from "react"; | |
| import React from "react"; | |
| import { useSearchParams, usePathname, useRouter } from "next/navigation"; | |
| import { useMemo, useCallback, useEffect } from "react"; | |
| import React from "react"; |
🤖 Prompt for AI Agents
In `@apps/web/modules/bookings/components/BookerWebWrapper.tsx` around lines 4 -
6, The component BookerWebWrapper uses router but removed the useRouter hook;
re-add the import "useRouter" from "next/navigation" and declare a router
instance inside the component (e.g., const router = useRouter()) alongside other
hooks (useSearchParams, usePathname, useMemo, etc.) so the existing references
to router (used in callbacks and effects) resolve and follow React hook rules.
Benchmark PR from agentic-review-benchmarks#9
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.