Async import of the appStore packages

[ShashankFC]

Test 2nn---nReplicated from ai-code-review-evaluation/cal.com-greptile#2

[github-actions]

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "Async import of the appStore packages". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

[ShashankFC]

@claude

[claude]

Claude finished @ShashankFC's task in 1m 45s —— View job

---

Code Review Complete

Todo List

• Check for repository-specific guidelines (CLAUDE.md)
• Review the PR diff and changed files
• Analyze async import implementation
• Check for potential bugs or issues
• Provide comprehensive feedback

---

Overview

This PR converts the app-store package imports from synchronous to asynchronous dynamic imports, which is a good performance optimization. However, there are several critical bugs that need to be addressed.

🔴 Critical Issues

1. Async forEach Anti-Pattern (Multiple Locations)

Several files use async callbacks in forEach loops, which doesn't properly handle promises:

packages/app-store/vital/lib/reschedule.ts:125-129 and packages/app-store/wipemycalother/lib/reschedule.ts:125-129:

bookingRefsFiltered.forEach(async (bookingRef) => {
  if (bookingRef.uid) {
    if (bookingRef.type.endsWith("_calendar")) {
      const calendar = await getCalendar(credentialsMap.get(bookingRef.type));
      return calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent);

Problem: forEach doesn't wait for async callbacks, so these operations run in parallel without proper error handling or completion guarantees. The return statements inside forEach also don't do what you'd expect.

Solution: Use for...of loop or Promise.all() with map():

await Promise.all(
  bookingRefsFiltered.map(async (bookingRef) => {
    if (bookingRef.uid) {
      if (bookingRef.type.endsWith("_calendar")) {
        const calendar = await getCalendar(credentialsMap.get(bookingRef.type));
        return calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent);
      }
      // ... rest of logic
    }
  })
);

Fix this →

packages/trpc/server/routers/viewer/bookings.tsx:553-563:

bookingRefsFiltered.forEach(async (bookingRef) => {
  if (bookingRef.uid) {
    if (bookingRef.type.endsWith("_calendar")) {
      const calendar = await getCalendar(credentialsMap.get(bookingRef.type));

Same issue here - needs to be converted to Promise.all() with map().

Fix this →

2. Missing await in handleCancelBooking.ts

packages/features/bookings/lib/handleCancelBooking.ts:461-463:

bookingToDelete.user.credentials
  .filter((credential) => credential.type.endsWith("_calendar"))
  .forEach(async (credential) => {
    const calendar = await getCalendar(credential);

Same anti-pattern, but this one is particularly concerning because it's in the critical booking cancellation flow. Calendar events might not be properly deleted if this isn't awaited.

Fix this →

⚠️ Important Issues

3. Race Condition in Sequential Operations

packages/features/bookings/lib/handleCancelBooking.ts:243:

if (reference.type.includes("_calendar")) {
  const calendar = await getCalendar(credential);
  if (calendar) {
    integrationsToDelete.push(
      calendar?.deleteEvent(reference.uid, evt, reference.externalCalendarId)

The code gets the calendar, then pushes the deletion promise to an array. However, if getCalendar() can now fail to resolve (dynamic import could fail), there's no error handling. Consider wrapping in try-catch.

4. Potential Type Inconsistency

packages/core/CalendarManager.ts:28:

const calendar = getCalendar(credential);
return app.variant === "calendar" ? [{ integration: app, credential, calendar }] : [];

The calendar is now a Promise<Calendar | null> rather than Calendar | null, which is stored in the object. This is later awaited correctly at line 48, but it's a bit confusing. Consider documenting this or awaiting immediately.

💡 Suggestions

5. Error Handling for Dynamic Imports

Dynamic imports can fail (module not found, syntax errors, etc.). Consider adding error handling around the app store access:

try {
  const app = await appStore[appName as keyof typeof appStore];
} catch (error) {
  log.error(`Failed to load app: ${appName}`, error);
  return null;
}

6. PR Title Issue

As noted by the bot, the PR title needs a Conventional Commits prefix. Based on the changes, this should be:

refactor: Async import of the appStore packages

or possibly:

perf: Async import of the appStore packages

✅ Positive Observations

1. Good Performance Optimization: Converting to dynamic imports is excellent for reducing initial bundle size and improving load times.

2. Consistent Pattern: The refactoring is applied consistently across the codebase.

3. Type Safety Maintained: The return type changes from Calendar | null to Promise<Calendar | null> are properly reflected.

4. Better Code Organization: The simplified index.ts in app-store is much cleaner.

📋 Summary

Must Fix Before Merge:

• All async callbacks in forEach loops (4 locations)
• Missing error handling for dynamic imports

Should Fix:

• PR title to follow Conventional Commits
• Add try-catch around critical dynamic import operations

Testing Recommendations:

• Test booking cancellation flows thoroughly
• Test with apps that might fail to load
• Verify all calendar integrations still work correctly

---

| Branch: appstore-async-improvements

[github-actions]

This PR is being marked as stale due to inactivity.