Async import of the appStore packages - #10
Conversation
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
There was a problem hiding this comment.
Walkthrough
This PR refactors the app store architecture from synchronous to asynchronous module loading by implementing dynamic imports and lazy loading. The core change converts appStore property access and getCalendar() function to async operations, requiring await keywords throughout the codebase. This enables code splitting and on-demand loading of app modules (calendar, video, payment integrations), improving initial bundle size and startup performance. The refactoring touches calendar management, event handling, video clients, booking operations, and payment processing. Several files also fix improper async handling in forEach loops, though some potential issues remain where forEach is used with async callbacks instead of proper async iteration patterns.
Changes
| File(s) | Summary |
|---|---|
packages/app-store/index.ts |
Refactored from static imports to dynamic imports for all 29 app modules, enabling lazy loading and code splitting while maintaining backward compatibility. |
packages/app-store/_utils/getCalendar.ts |
Converted getCalendar function to async, changing return type from Calendar | null to Promise<Calendar | null> and added await for appStore access. |
packages/core/CalendarManager.ts |
Updated all getCalendar() calls to async/await across getConnectedCalendars(), getCachedResults(), createEvent(), updateEvent(), and deleteEvent(). Made deleteEvent() async and wrapped calendar promises in Promise.all() in getCachedResults(). |
packages/core/EventManager.ts |
Added await to getCalendar() call to properly handle Promise before deleting calendar events. |
packages/core/videoClient.ts |
Converted getVideoAdapters factory function to async using for...of loop instead of reduce. Updated all call sites (getBusyVideoTimes, createMeeting, updateMeeting, deleteMeeting, createMeetingWithCalVideo, getRecordingsOfCalVideoByRoomName, getDownloadLinkOfCalVideoByRecordingId) to await the function. |
packages/features/bookings/lib/handleCancelBooking.ts |
Added await to all five getCalendar() calls and appStore access for payment app retrieval. Refactored forEach loop to for...of loop for proper async handling of calendar credentials. |
packages/features/bookings/lib/handleNewBooking.ts |
Added await to getCalendar(credential) call in booking cancellation/rescheduling flow. |
packages/lib/payment/deletePayment.ts |
|
packages/lib/payment/handlePayment.ts |
Added await to appStore property access when retrieving payment app instances. |
packages/app-store/vital/lib/reschedule.ts |
|
packages/app-store/wipemycalother/lib/reschedule.ts |
|
packages/trpc/server/routers/viewer/bookings.tsx |
Made forEach callbacks async and added await to getCalendar() calls. Note: Using async callbacks with forEach may require refactoring to Promise.all() with map() or for...of loop for proper async handling. |
Sequence Diagram
This diagram shows the interactions between components:
sequenceDiagram
participant Caller
participant CalendarManager
participant AppStore
participant CalendarApp
participant CalendarService
Caller->>CalendarManager: getCalendar(credential)
alt credential is null or has no key
CalendarManager-->>Caller: return null
else credential is valid
CalendarManager->>CalendarManager: Extract and process calendarType
Note over CalendarManager: Remove "_other_calendar" suffix if present
CalendarManager->>AppStore: await appStore[calendarType]
activate AppStore
AppStore-->>CalendarManager: calendarApp
deactivate AppStore
alt calendarApp has lib.CalendarService
CalendarManager->>CalendarApp: Access CalendarService
CalendarApp-->>CalendarManager: CalendarService instance
CalendarManager-->>Caller: return Calendar
else calendarApp missing CalendarService
Note over CalendarManager: Log warning about<br/>unimplemented calendar type
CalendarManager-->>Caller: return null
end
end
🔗 Cross-Repository Impact Analysis
Enable automatic detection of breaking changes across your dependent repositories. → Set up now
Learn more about Cross-Repository Analysis
What It Does
- Automatically identifies repositories that depend on this code
- Analyzes potential breaking changes across your entire codebase
- Provides risk assessment before merging to prevent cross-repo issues
How to Enable
- Visit Settings → Code Management
- Configure repository dependencies
- Future PRs will automatically include cross-repo impact analysis!
Benefits
- 🛡️ Prevent breaking changes across repositories
- 🔍 Catch integration issues before they reach production
- 📊 Better visibility into your multi-repo architecture
Install the extension
Note for Windsurf
Please change the default marketplace provider to the following in the windsurf settings:Marketplace Extension Gallery Service URL: https://marketplace.visualstudio.com/_apis/public/gallery
Marketplace Gallery Item URL: https://marketplace.visualstudio.com/items
Entelligence.ai can learn from your feedback. Simply add 👍 / 👎 emojis to teach it your preferences. More shortcuts below
Emoji Descriptions:
⚠️ Potential Issue - May require further investigation.- 🔒 Security Vulnerability - Fix to ensure system safety.
- 💻 Code Improvement - Suggestions to enhance code quality.
- 🔨 Refactor Suggestion - Recommendations for restructuring code.
- ℹ️ Others - General comments and information.
Interact with the Bot:
- Send a message or request using the format:
@entelligenceai + *your message*
Example: @entelligenceai Can you suggest improvements for this code?
- Help the Bot learn by providing feedback on its responses.
@entelligenceai + *feedback*
Example: @entelligenceai Do not comment on `save_auth` function !
Also you can trigger various commands with the bot by doing
@entelligenceai command
The current supported commands are
config- shows the current configretrigger_review- retriggers the review
More commands to be added soon.
| applecalendar: import("./applecalendar"), | ||
| caldavcalendar: import("./caldavcalendar"), | ||
| closecom: import("./closecom"), | ||
| dailyvideo: import("./dailyvideo"), | ||
| googlecalendar: import("./googlecalendar"), | ||
| googlevideo: import("./googlevideo"), | ||
| hubspot: import("./hubspot"), | ||
| huddle01video: import("./huddle01video"), | ||
| jitsivideo: import("./jitsivideo"), | ||
| larkcalendar: import("./larkcalendar"), | ||
| office365calendar: import("./office365calendar"), | ||
| office365video: import("./office365video"), | ||
| plausible: import("./plausible"), | ||
| salesforce: import("./salesforce"), | ||
| zohocrm: import("./zohocrm"), | ||
| sendgrid: import("./sendgrid"), | ||
| stripepayment: import("./stripepayment"), | ||
| tandemvideo: import("./tandemvideo"), | ||
| vital: import("./vital"), | ||
| zoomvideo: import("./zoomvideo"), | ||
| wipemycalother: import("./wipemycalother"), | ||
| giphy: import("./giphy"), | ||
| zapier: import("./zapier"), | ||
| exchange2013calendar: import("./exchange2013calendar"), | ||
| exchange2016calendar: import("./exchange2016calendar"), | ||
| exchangecalendar: import("./exchangecalendar"), | ||
| facetime: import("./facetime"), | ||
| sylapsvideo: import("./sylapsvideo"), | ||
| }; |
There was a problem hiding this comment.
Correctness: In appStore, replacing static imports with import() expressions changes the property values from module namespaces to Promise objects. This breaks all synchronous consumers of this export. Revert to static imports or, if lazy loading is intended, use factory functions (e.g., applecalendar: () => import("./applecalendar")) and update all call sites to handle the asynchronous resolution.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
File: packages/app-store/index.ts (lines 3-31). The appStore entries now use dynamic import, which returns Promise and breaks synchronous consumers. Decide on the contract: (1) revert to static imports to preserve sync usage, or (2) change entries to lazy loader functions `() => import("./...")` and update all call sites to await the loader before accessing module exports.
| const connectedCalendars = await Promise.all( | ||
| calendarCredentials.map(async (item) => { | ||
| try { | ||
| const { calendar, integration, credential } = item; | ||
|
|
||
| const { integration, credential } = item; | ||
| const calendar = await item.calendar; | ||
| // Don't leak credentials to the client | ||
| const credentialId = credential.id; | ||
| if (!calendar) { |
There was a problem hiding this comment.
Correctness: In getConnectedCalendars, the if (!calendar) block (line 49) returns the raw integration object, which contains sensitive app credentials (e.g., access tokens). This security leak was previously unreachable because item.calendar was a Promise (always truthy), but the change to await item.calendar makes this path active. Wrap integration with cleanIntegrationKeys(integration) in this return statement to prevent leaking credentials to the client.
| bookingRefsFiltered.forEach(async (bookingRef) => { | ||
| if (bookingRef.uid) { | ||
| if (bookingRef.type.endsWith("_calendar")) { | ||
| const calendar = getCalendar(credentialsMap.get(bookingRef.type)); | ||
| const calendar = await getCalendar(credentialsMap.get(bookingRef.type)); | ||
| return calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent); |
There was a problem hiding this comment.
Correctness: The bookingRefsFiltered.forEach call does not await the async callback. This results in fire-and-forget execution where the surrounding try/catch block fails to catch rejections, and the function proceeds to send emails before deletions are finished. Replace this with a for...of loop or await Promise.all() to ensure all operations are properly awaited and errors are caught.
Affected Locations:
- packages/app-store/vital/lib/reschedule.ts:125-129
- packages/app-store/wipemycalother/lib/reschedule.ts:124-131
- packages/trpc/server/routers/viewer/bookings.tsx:552-552
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
File: packages/app-store/vital/lib/reschedule.ts. Replace the `bookingRefsFiltered.forEach(async ...)` block (lines ~125-129) with an awaited `Promise.all(bookingRefsFiltered.map(async ...))` (or a `for...of` with `await`) so deletions are awaited and errors are caught by the surrounding try/catch. Keep the existing logic inside the loop unchanged.
2 participants
Test 2nn
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.
nn---n*Replicated from [ai-code-review-evaluation/cal.com-coderabbit#2](https://github.com/ai-code-review-evaluation/cal.com-coderabbit/pull/2)*EntelligenceAI PR Summary
Refactored app store architecture from synchronous to asynchronous module loading using dynamic imports for improved performance and code splitting.
getCalendar()function andappStoreaccess to async operations requiringawaitthroughout codebaseimport()calls in app store indexgetConnectedCalendars,getCachedResults,createEvent,updateEvent,deleteEvent) to handle async calendar retrievalgetVideoAdaptersfactory function to async and updated all video client call sitesawaitto calendar and payment app retrievals in booking handlers and payment processingforEachloops tofor...offor proper async handling, though some files still use async callbacks withforEach(potential issue)