refactor: move WebWrapper files from packages/platform to apps/web/modules #20
Conversation
WalkthroughThis PR reorganizes and consolidates import paths for web components and utility functions across the codebase. Components previously exported from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/modules/bookings/components/BookerWebWrapper.tsx (1)
183-196:routeris undefined - this callback will fail at runtime.The
onOverlaySwitchStateChangecallback referencesrouterwhich is no longer defined after the import removal. This will cause the overlay calendar toggle functionality to break.
🤖 Fix all issues with AI agents
In `@apps/web/modules/bookings/components/BookerWebWrapper.tsx`:
- Around line 4-5: The component still references router in functions like
onOverlaySwitchStateChange, and in places calling router.push (e.g., inside
BookerWebWrapperComponent and handlers that build newPath/newUrl), but useRouter
was removed from imports; re-import useRouter from next/navigation and
initialize const router = useRouter() inside BookerWebWrapperComponent
(alongside existing usePathname/useSearchParams) so all uses of router
(router.push calls and dependency arrays) are valid; ensure dependency arrays
that included [router] still reference the local router variable.
🧹 Nitpick comments (4)
apps/web/modules/event-types/components/tabs/limits/EventLimitsTabWebWrapper.tsx (1)
4-6: Consider whether this wrapper is necessary.The wrapper component is a simple pass-through that adds no additional logic or behavior. If there's no plan to extend this wrapper with web-specific functionality, consider directly exporting
EventLimitsTabinstead.♻️ Simplification option
If the wrapper serves no architectural purpose:
-import { EventLimitsTab } from "@calcom/web/modules/event-types/components/tabs/limits/EventLimitsTab"; -import type { EventLimitsTabProps } from "@calcom/web/modules/event-types/components/tabs/limits/EventLimitsTab"; - -const EventLimitsTabWebWrapper = (props: EventLimitsTabProps) => { - return <EventLimitsTab {...props} />; -}; - -export default EventLimitsTabWebWrapper; +export { EventLimitsTab as default } from "@calcom/web/modules/event-types/components/tabs/limits/EventLimitsTab";packages/lib/bookings/hostGroupUtils.ts (1)
41-60: LGTM — consider adding a brief JSDoc comment.The centralized
sortHostsutility handles null defaults appropriately and correctly implements dual-mode sorting. The sorting logic differs intentionally between weight-enabled and weight-disabled modes (ascending vs descending priority). A short JSDoc explaining the expected sort order for each mode would help future maintainers.📝 Optional: Add documentation for sort behavior
+/** + * Sorts hosts for round-robin assignment. + * - When RR weights enabled: higher weight first, then lower priority value first (ties). + * - When RR weights disabled: higher priority value first. + */ 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/tabs/availability/EventAvailabilityTab.tsx (1)
8-8: Potential circular import between sibling modules.This creates a circular dependency:
EventAvailabilityTabWebWrapperimportsEventAvailabilityTabEventAvailabilityTabimportsGetAllSchedulesByUserIdQueryTypefromEventAvailabilityTabWebWrapperWhile this may work for type-only imports with TypeScript's
isolatedModules, it can cause issues with certain bundlers or at runtime. Consider extracting shared types to a dedicatedtypes.tsfile in this directory.♻️ Suggested refactor: Extract shared types
Create
apps/web/modules/event-types/components/tabs/availability/types.ts:import type { trpc } from "@calcom/trpc/react"; export type GetAllSchedulesByUserIdQueryType = typeof trpc.viewer.availability.schedule.getAllSchedulesByUserId.useQuery;Then update both files to import from
./types:-import type { GetAllSchedulesByUserIdQueryType } from "./EventAvailabilityTabWebWrapper"; +import type { GetAllSchedulesByUserIdQueryType } from "./types";apps/web/modules/event-types/components/EventTypeWebWrapper.tsx (1)
24-31: Import ordering could be improved for consistency.The imports mix package paths (
@calcom/web,@calcom/atoms) with relative paths. Consider grouping them consistently:
- External packages (
next/...,zod)- Internal packages (
@calcom/...)- Relative imports (
./...)♻️ Suggested reordering
-import { revalidateTeamEventTypeCache } from "@calcom/web/app/(booking-page-wrapper)/team/[slug]/[type]/actions"; -import { revalidateEventTypeEditPage } from "@calcom/web/app/(use-page-wrapper)/event-types/[type]/actions"; - -import type { ChildrenEventType } from "./ChildrenEventTypeSelect"; -import { EventType as EventTypeComponent } from "./EventType"; import { useEventTypeForm } from "@calcom/atoms/event-types/hooks/useEventTypeForm"; import { useHandleRouteChange } from "@calcom/atoms/event-types/hooks/useHandleRouteChange"; import { useTabsNavigations } from "@calcom/atoms/event-types/hooks/useTabsNavigations"; +import { revalidateTeamEventTypeCache } from "@calcom/web/app/(booking-page-wrapper)/team/[slug]/[type]/actions"; +import { revalidateEventTypeEditPage } from "@calcom/web/app/(use-page-wrapper)/event-types/[type]/actions"; + +import type { ChildrenEventType } from "./ChildrenEventTypeSelect"; +import { EventType as EventTypeComponent } from "./EventType";
📜 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 (27)
apps/web/modules/event-types/components/tabs/recurring/EventRecurringWebWrapper.tsx (1)
4-8: LGTM — no functional change here.Wrapper remains a simple passthrough; nothing to adjust.
apps/web/modules/apps/components/ConferencingAppsViewWebWrapper.tsx (1)
14-15: LGTM — local imports align with refactor.apps/web/app/(use-page-wrapper)/settings/(settings-layout)/my-account/conferencing/loading.tsx (1)
1-1: LGTM — import path matches new module location.apps/web/app/(use-page-wrapper)/settings/(settings-layout)/my-account/conferencing/page.tsx (1)
4-4: LGTM — import path matches new module location.apps/web/modules/event-types/components/AddMembersWithSwitch.tsx (1)
6-7: LGTM - Import path refactoring is correct.The relative import aligns with the new local file structure. Note there's a circular dependency between this file and
AddMembersWithSwitchWebWrapper.tsx(each imports from the other). This pattern is common for wrapper/base component relationships and works fine with ES modules since the component references aren't evaluated until render time.apps/web/modules/event-types/components/AddMembersWithSwitchWebWrapper.tsx (1)
3-5: LGTM - Local imports are correctly configured.Using
import typeforAddMembersWithSwitchPropsis good practice as it ensures the type import is erased at compile time, which helps mitigate potential circular dependency issues at runtime.packages/platform/atoms/event-types/hooks/useEventTypeForm.ts (1)
11-11: LGTM!The import path correctly points to the centralized
sortHostsutility inhostGroupUtils.apps/web/modules/event-types/components/tabs/setup/EventSetupTabWebWrapper.tsx (1)
6-8: LGTM!Relative imports are appropriate for co-located files within the same
tabs/setup/directory.apps/web/modules/event-types/components/tabs/availability/EventAvailabilityTabWebWrapper.tsx (1)
8-10: LGTM!Import paths correctly updated to use relative paths within the consolidated module structure.
apps/web/modules/event-types/components/EventTypeWebWrapper.tsx (2)
48-91: LGTM!Dynamic imports correctly updated to reference the consolidated tab wrapper locations under
./tabs/.
234-250: LGTM!Tab rendering is unchanged functionally; formatting adjustments are consistent.
apps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTabWebWrapper.tsx (1)
1-4: Relative import is appropriate for co-located modules.Using a relative import (
./EventTeamAssignmentTab) for files in the same directory is cleaner than the external alias path and reduces dependency on module alias configuration.apps/web/app/(use-page-wrapper)/event-types/[type]/page.tsx (1)
9-9: Import path update correctly reflects component migration.The import path change to
@calcom/web/modules/event-types/components/EventTypeWebWrapperis valid. The component exists at the new location and the old path is no longer in the codebase.packages/platform/atoms/package.json (1)
59-75: Package exports updated to reflect WebWrapper migration.The export changes correctly remove WebWrapper references that have been migrated to
@calcom/web/modules, while adding new exports for the event-types hooks (useEventTypeForm,useHandleRouteChange,useTabsNavigations) and theSelectedCalendarsSettingscomponent. No orphaned imports of removed WebWrapper exports exist in the codebase.apps/web/modules/event-types/components/HostEditDialogs.tsx (1)
13-13: Host utilities properly consolidated into shared module.The import of
groupHostsByGroupId,getHostsFromOtherGroups, andsortHostsfrom@calcom/lib/bookings/hostGroupUtilsis correct. All three functions are properly exported from the utility file. This consolidation improves maintainability by centralizing host-related logic in a single source of truth.apps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTab.tsx (1)
17-17: Import centralized to shared utility location.Moving
sortHoststo@calcom/lib/bookings/hostGroupUtilscentralizes the host-related utility for reuse across the codebase. The function is properly exported and used consistently at lines 324 and 332.apps/web/modules/calendars/components/SelectedCalendarsSettingsWebWrapper.tsx (1)
15-15: LGTM!The import path update to
@calcom/atoms/selected-calendars/SelectedCalendarsSettingsaligns with the broader module reorganization in this PR.apps/web/modules/event-types/components/tabs/advanced/EventAdvancedTab.tsx (1)
9-13: LGTM!The import path update to
@calcom/web/modules/calendars/components/SelectedCalendarsSettingsWebWrapperis consistent with the module reorganization pattern used across this PR.apps/web/modules/event-types/components/tabs/advanced/EventAdvancedWebWrapper.tsx (1)
3-5: LGTM!Using relative imports (
./EventAdvancedTab) for co-located components is appropriate and aligns with the local module structure approach in this PR.apps/web/components/apps/CalendarListContainer.tsx (1)
6-7: LGTM!The import path updates are appropriate:
- Relative import for co-located
DestinationCalendarSettingsWebWrapper- Package-scoped import for
SelectedCalendarsSettingsWebWrapperfrom the web modules namespaceapps/web/modules/bookings/components/BookerWebWrapper.tsx (1)
35-36: LGTM for the Booker import change.The import path update to a local
./Bookercomponent is consistent with the PR's module reorganization objectives.apps/web/components/apps/DestinationCalendarSettingsWebWrapper.tsx (1)
3-4: Use the public API exports from@calcom/atomsinstead of deep relative paths.The deep relative paths here traverse directly into the package structure, which is fragile. However, note that the suggested imports in the original review would not work—these paths are not exported from
@calcom/atoms.The file should import from the public API:
DestinationCalendarSettingsis properly exported from@calcom/atomsAtomsWrapperis not part of the public API and should be either added to the public exports or its usage reconsideredVerify the correct import path for
AtomsWrapper(it may be an internal component) and update both imports to use the package alias:-import { AtomsWrapper } from "../../../../packages/platform/atoms/src/components/atoms-wrapper"; -import { DestinationCalendarSettings } from "../../../../packages/platform/atoms/destination-calendar/DestinationCalendar"; +import { DestinationCalendarSettings } from "@calcom/atoms";For
AtomsWrapper, confirm whether it should be part of the public API or if there's an alternative approach.Likely an incorrect or invalid review comment.
apps/web/modules/org/[orgSlug]/instant-meeting/team/[slug]/[type]/instant-meeting-view.tsx (1)
5-5: LGTM — import path refactor only.No behavioral changes; usage remains consistent.
apps/web/modules/users/views/users-type-public-view.tsx (1)
6-6: LGTM — import path refactor only.No behavioral or prop changes observed.
apps/web/modules/d/[link]/d-type-view.tsx (1)
3-3: LGTM — import path refactor only.No runtime changes expected.
apps/web/modules/team/type-view.tsx (1)
6-6: LGTM — import path refactor only.No logic changes in this segment.
apps/web/modules/team/type-view-cached.tsx (1)
3-3: LGTM — type import path refactor only.No behavior or API changes in this file.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| import { useSearchParams, usePathname } from "next/navigation"; | ||
| import { useMemo, useCallback, useEffect } from "react"; |
There was a problem hiding this comment.
Critical: useRouter was removed but router is still used throughout the component.
The import for useRouter from next/navigation was removed, but the router variable is still used in multiple places:
- Line 193:
router.push(...)inonOverlaySwitchStateChange - Line 195:
[router]in dependency array - Line 228:
router.push(newPath) - Line 232:
router.push("/apps/categories/calendar") - Line 238:
router.push(newUrl.toString())
This will cause runtime errors (ReferenceError: router is not defined) when users interact with the overlay calendar, instant meeting, or navigation features.
Proposed fix: Restore the useRouter import and usage
-import { useSearchParams, usePathname } from "next/navigation";
-import { useMemo, useCallback, useEffect } from "react";
+import { useSearchParams, usePathname, useRouter } from "next/navigation";
+import { useMemo, useCallback, useEffect } from "react";Then add the router initialization inside BookerWebWrapperComponent:
const BookerWebWrapperComponent = (props: BookerWebWrapperAtomProps) => {
const pathname = usePathname();
const searchParams = useSearchParams();
const router = useRouter(); // Add this line
// ... rest of component🤖 Prompt for AI Agents
In `@apps/web/modules/bookings/components/BookerWebWrapper.tsx` around lines 4 -
5, The component still references router in functions like
onOverlaySwitchStateChange, and in places calling router.push (e.g., inside
BookerWebWrapperComponent and handlers that build newPath/newUrl), but useRouter
was removed from imports; re-import useRouter from next/navigation and
initialize const router = useRouter() inside BookerWebWrapperComponent
(alongside existing usePathname/useSearchParams) so all uses of router
(router.push calls and dependency arrays) are valid; ensure dependency arrays
that included [router] still reference the local router variable.
Benchmark PR from qodo-benchmark#708
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.