perf: implement quick app store loading optimizations#22450
Conversation
- Add conditional app store imports in videoClient and handlePayment - Implement lazy calendar manager pattern in CalendarManager - Enhance createCachedImport with better concurrency handling - Create calendar-only registry for common calendar operations - Add performance instrumentation for debugging These optimizations reduce initial app store loading time by avoiding module-level imports and creating smaller, focused registries for calendar operations. Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
✅ No security or compliance issues detected. Reviewed everything up to 48a909e. Security Overview
Detected Code Changes
Reply to this PR with |
E2E results are ready! |
- Reverted getCalendar.ts to use main appStore instead of calendarStore - This ensures test mocking system works properly with existing appStoreMock - Fixes unit test failures where Google Calendar references were getting null values - All collective scheduling tests now pass Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
- Add CalendarServiceMap generation following CrmServiceMap pattern - Update getCalendar.ts to use generated calendar service map - Remove manual calendar-registry.ts in favor of auto-generated approach - Reduces calendar initialization from loading 48+ apps to ~10 calendar apps Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
WalkthroughThe PR generates and exports a CalendarServiceMap (packages/app-store/calendar.services.generated.ts, empty under NEXT_PUBLIC_IS_E2E), refactors runtime calendar resolution to use that map (getCalendar), and changes calendar test helpers to an async, map-driven mocking API (mockCalendar and wrappers now return Promises). Tests were updated to await calendar mocks and several expectations dropped meetingUrl. CalendarManager now stores calendars as thunks that call getCalendar and handles missing instances explicitly. Multiple modules switched from static to dynamic app-store imports and two location constants were centralized. Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
- Add missing SelectedCalendar fields (createdAt, updatedAt, lastErrorAt, watchAttempts, etc.) - Fix CredentialPayload type errors by adding user.email and delegationCredentialId - Mock CalendarServiceMap to use vi.importActual for real calendar services - Ensure calendar service tests work with new lazy loading approach Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
- Update getCalendarsEvents.test.ts mocks to work with CalendarServiceMap dynamic imports - Add missing SelectedCalendar fields (createdAt, updatedAt, lastErrorAt, watchAttempts, etc.) - Fix CredentialPayload type errors by adding user.email and delegationCredentialId - Use type assertion in getCalendar.ts to resolve credential type conflicts - Ensure calendar service tests work with new lazy loading approach Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
…eMap - Resolve credential object merge conflicts keeping user email field - Add missing SelectedCalendar fields (createdAt, updatedAt, etc.) - Ensure CredentialPayload compatibility with delegationCredentialId - Verify CalendarServiceMap mocking works with dynamic imports - All 17 tests now passing locally with performance improvements Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
- Add comprehensive vi.mock for calendar.services.generated in delegation-credential tests - Mock GoogleCalendarService and Office365CalendarService with proper return values - Update all test files to use await with mockCalendarToHaveNoBusySlots - Ensure calendar events return expected meetingId, meetingPassword, meetingUrl values - Fix async/await compatibility issues in booking scenario test utilities Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
- Extract CalendarServiceMap promise to variable to fix 'always true' condition - Ensure vi.mocked is called on Promise type for proper mockResolvedValue access - Add await keywords to calendar mock calls in test files - Maintain existing functionality while making code type-safe Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
|
This PR is being marked as stale due to inactivity. |
…ub.com:calcom/cal.com into devin/app-store-quick-optimizations-1752336742
Revert the removal in app-store/index.ts also.
![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
🧹 Nitpick comments (4)
packages/app-store/index.ts (4)
50-73: Streamline loader state: rely on a single promise, harden against sync-throws, and avoid state driftUsing both isLoading and loadingPromise is redundant and increases the chance of state drift. A single loadingPromise is sufficient for deduplication. Also, wrapping importFunc with Promise.resolve guards against a (rare) synchronous throw. The refactor below simplifies control flow, removes double writes to isLoading, and ensures we always return a non-undefined Promise reference.
- let isLoading = false; - let loadingPromise: Promise<T> | undefined; + let loadingPromise: Promise<T> | undefined; return async () => { - if (cachedModule) return cachedModule; - if (isLoading && loadingPromise) return loadingPromise; - - isLoading = true; - loadingPromise = importFunc() - .then((module) => { - cachedModule = module; - isLoading = false; - return module; - }) - .catch((err) => { - throw err; - }) - .finally(() => { - isLoading = false; - loadingPromise = undefined; - }); - - return loadingPromise; + if (cachedModule) return cachedModule; + if (loadingPromise) return loadingPromise; + + const p = Promise.resolve(importFunc()) + .then((module) => { + cachedModule = module; + return module; + }) + .finally(() => { + // Clear to allow retries after failures and free the reference after success. + loadingPromise = undefined; + }); + loadingPromise = p; + return p; };
57-71: Nit: duplicate state mutation; letfinallyhandle cleanup
isLoading = false;is set in both.thenand.finally. The.finallyis sufficient and avoids double writes. Also consider wrapping the import withPromise.resolve(...)to catch any sync throws (rare but cheap to guard).- loadingPromise = importFunc() + loadingPromise = Promise.resolve(importFunc()) .then((module) => { cachedModule = module; - isLoading = false; return module; }) .catch((err) => { throw err; }) .finally(() => { isLoading = false; loadingPromise = undefined; });
80-82: Make the mock app gate explicit: enable only on “true”
process.env.MOCK_PAYMENT_APP_ENABLED !== undefinedwill enable the mock even if set to an empty string. Prefer an explicit true check to prevent accidental activation across environments.-if (process.env.MOCK_PAYMENT_APP_ENABLED !== undefined) { +if (process.env.MOCK_PAYMENT_APP_ENABLED?.toLowerCase() === "true") { exportedAppStore["mock-payment-app"] = createCachedImport(() => import("./mock-payment-app/index")); }If you’d like, I can open a follow-up to document the expected values for this env var across local/dev/prod.
84-84: Add a non-breaking named export forappStorePer repository conventions, exposing a named export improves clarity and enables tree-shaking. We’ve audited all
@calcom/app-storeimports—there are numerous default imports and zero existing named imports ofappStore, so this change is safe and conflict-free.• In packages/app-store/index.ts (around line 84)
+export { exportedAppStore as appStore }; export default exportedAppStore;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/app-store/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/app-store/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/app-store/index.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/app-store/index.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.
| loadingPromise = undefined; | ||
| }); | ||
|
|
||
| return loadingPromise; |
There was a problem hiding this comment.
Type-safety: return value can be undefined under TS’s eyes
With let loadingPromise: Promise<T> | undefined, return loadingPromise; may fail type-checking (strictNullChecks) because TS can’t guarantee non-undefined here. Either use a non-null assertion or (preferably) adopt the refactor above that returns a local p.
Minimal fix if you keep the current structure:
- return loadingPromise;
+ return loadingPromise!;📝 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.
| return loadingPromise; | |
| return loadingPromise!; |
🤖 Prompt for AI Agents
In packages/app-store/index.ts around line 72, TypeScript reports the return
value may be undefined because loadingPromise is declared as Promise<T> |
undefined and returned directly; either assert non-null when returning (e.g.,
return loadingPromise!;) or, preferably, refactor to assign the created promise
to a local const (const p = loadingPromise = new Promise<T>(...);) and return
that local p so the compiler can see a definite Promise<T> is returned.
…ub.com:calcom/cal.com into devin/app-store-quick-optimizations-1752336742
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (9)
packages/app-store/applecalendar/lib/CalendarService.ts (3)
1-1: Consider switching to a named import for the base class to avoid default exports.Repo guideline discourages default exports. If/when
@calcom/lib/CalendarServiceexposes a named export for the base class, preferimport { BaseCalendarService } from "@calcom/lib/CalendarService";. Not a blocker for this perf PR but worth planning to reduce refactor friction and improve tree-shaking.
4-8: Prefer named export for AppleCalendarService (avoid default exports).To align with the repo guideline, consider exporting the class by name. This will also make static import sites cleaner and more consistent with the new generated map.
Apply in a follow-up PR (generator, mocks, and import sites likely assume default export today):
-export default class AppleCalendarService extends BaseCalendarService { +export class AppleCalendarService extends BaseCalendarService {
5-7: De-duplicate service identifiers via a shared constant/enforced type.Hardcoded
"apple_calendar"risks drift with the generated CalendarServiceMap keys. Consider centralizing IDs (e.g., an enum or const object) and typing the constructor param to that union to prevent typos.packages/app-store/caldavcalendar/lib/CalendarService.ts (3)
1-1: Prefer named import for BaseCalendarService (avoid default export).Same note as in Apple: when feasible, switch to
import { BaseCalendarService } ...to eliminate default exports in this module.
4-8: Avoid default export for CalDavCalendarService.Prefer a named export to follow the repo guideline and keep generated/static imports consistent in the long term.
-export default class CalDavCalendarService extends BaseCalendarService { +export class CalDavCalendarService extends BaseCalendarService {
6-6: Align service ID with a central constant/type.
"caldav_calendar"should likely come from a typed constant shared with the generator to prevent mismatches.setupVitest.ts (3)
16-35: DRY up identical mock event payloads in create/update.Both methods return the same object. Factor into a tiny helper to reduce duplication and cut future drift.
+const mockEvent = { + uid: "mock", + id: "mock", + password: "", + type: "", + url: "", + additionalInfo: {}, +} as const; async createEvent() { - return { - uid: "mock", - id: "mock", - password: "", - type: "", - url: "", - additionalInfo: {}, - }; + return { ...mockEvent }; } async updateEvent() { - return { - uid: "mock", - id: "mock", - password: "", - type: "", - url: "", - additionalInfo: {}, - }; + return { ...mockEvent }; }
37-42: Optional: tighten mock method signatures to catch breaking changes sooner.If you import the relevant types (e.g., CalendarEvent, AvailabilitySlot), add explicit return/param types to mocks. This makes test mocks act as sentinels if signatures change in the services.
45-55: Consolidate repeated mocks for Exchange variants.Minor cleanup: reduce repetition and avoid missing a variant in the future.
-vi.mock("@calcom/exchangecalendar/lib/CalendarService", () => ({ - default: MockExchangeCalendarService, -})); - -vi.mock("@calcom/exchange2013calendar/lib/CalendarService", () => ({ - default: MockExchangeCalendarService, -})); - -vi.mock("@calcom/exchange2016calendar/lib/CalendarService", () => ({ - default: MockExchangeCalendarService, -})); +const exchangeModules = [ + "@calcom/exchangecalendar/lib/CalendarService", + "@calcom/exchange2013calendar/lib/CalendarService", + "@calcom/exchange2016calendar/lib/CalendarService", +] as const; +for (const m of exchangeModules) { + vi.mock(m, () => ({ default: MockExchangeCalendarService })); +}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
packages/app-store/applecalendar/lib/CalendarService.ts(1 hunks)packages/app-store/caldavcalendar/lib/CalendarService.ts(1 hunks)setupVitest.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*Service.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/app-store/caldavcalendar/lib/CalendarService.tspackages/app-store/applecalendar/lib/CalendarService.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/app-store/caldavcalendar/lib/CalendarService.tspackages/app-store/applecalendar/lib/CalendarService.tssetupVitest.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/app-store/caldavcalendar/lib/CalendarService.tspackages/app-store/applecalendar/lib/CalendarService.tssetupVitest.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/app-store/caldavcalendar/lib/CalendarService.tspackages/app-store/applecalendar/lib/CalendarService.tssetupVitest.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.
📚 Learning: 2025-08-05T12:04:29.037Z
Learnt from: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.
Applied to files:
packages/app-store/caldavcalendar/lib/CalendarService.tspackages/app-store/applecalendar/lib/CalendarService.tssetupVitest.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
| vi.mock("@calcom/exchangecalendar/lib/CalendarService", () => ({ | ||
| default: MockExchangeCalendarService, | ||
| })); | ||
|
|
||
| vi.mock("@calcom/exchange2013calendar/lib/CalendarService", () => ({ | ||
| default: MockExchangeCalendarService, | ||
| })); | ||
|
|
||
| vi.mock("@calcom/exchange2016calendar/lib/CalendarService", () => ({ | ||
| default: MockExchangeCalendarService, | ||
| })); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure mocks are applied before static imports are evaluated.
With static imports replacing dynamic imports, it's critical that these vi.mock calls execute before any module imports the generated CalendarServiceMap or calendar services. Confirm setupVitest.ts is listed in setupFiles (or setupFilesAfterEnv) and runs before test modules load imports used by the generator.
If needed, verify Vitest config:
🏁 Script executed:
#!/bin/bash
# Show Vitest configs that declare setup files and workspace ordering
fd -t f -a 'vitest.config.*' | xargs -I{} sh -c 'echo "--- {} ---"; cat "{}"'Length of output: 870
Add setupVitest.ts to Vitest’s setupFiles so mocks run before any imports
The current vitest.config.ts doesn’t include your setupVitest.ts in either setupFiles or setupFilesAfterEnv, so those vi.mock(...) calls will execute too late (after static imports). You need to ensure Vitest loads your setup file before any test modules are imported.
– In vitest.config.ts, under the test block, add a setupFiles entry pointing to your setup file. For example:
export default defineConfig({
test: {
+ // Ensure mocks are applied before any module import
+ setupFiles: ["./setupVitest.ts"],
coverage: {
provider: "v8",
},
passWithNoTests: true,
testTimeout: 500000,
},
});• Adjust the path in setupFiles as needed to correctly locate your setupVitest.ts.
• After this change, Vitest will invoke your mocks before evaluating any static imports (including the generated CalendarServiceMap).
📝 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.
| vi.mock("@calcom/exchangecalendar/lib/CalendarService", () => ({ | |
| default: MockExchangeCalendarService, | |
| })); | |
| vi.mock("@calcom/exchange2013calendar/lib/CalendarService", () => ({ | |
| default: MockExchangeCalendarService, | |
| })); | |
| vi.mock("@calcom/exchange2016calendar/lib/CalendarService", () => ({ | |
| default: MockExchangeCalendarService, | |
| })); | |
| export default defineConfig({ | |
| test: { | |
| // Ensure mocks are applied before any module import | |
| setupFiles: ["./setupVitest.ts"], | |
| coverage: { | |
| provider: "v8", | |
| }, | |
| passWithNoTests: true, | |
| testTimeout: 500000, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In setupVitest.ts around lines 45 to 55, your vi.mock(...) calls will run too
late because vitest.config.ts does not include this file in setupFiles; update
vitest.config.ts's test block to add a setupFiles entry that points to the
setupVitest.ts path (adjust relative path as needed) so Vitest loads the setup
file before any test modules are imported and the mocks are applied before
static imports (no other code changes required).
- Add analytics service generation to app-store-cli build process - Generate analytics.services.generated.ts with only analytics apps (dub) - Update getAnalytics.ts to use AnalyticsServiceMap instead of full appStore - Add NEXT_PUBLIC_IS_E2E to turbo.json globalEnv for generated files - Reduces import footprint from 100+ apps to only analytics apps with AnalyticsService - Follows same pattern as calendar services optimization from PR #22450 Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
#23372) * perf: optimize analytics app imports to avoid loading entire app store - Add analytics service generation to app-store-cli build process - Generate analytics.services.generated.ts with only analytics apps (dub) - Update getAnalytics.ts to use AnalyticsServiceMap instead of full appStore - Add NEXT_PUBLIC_IS_E2E to turbo.json globalEnv for generated files - Reduces import footprint from 100+ apps to only analytics apps with AnalyticsService - Follows same pattern as calendar services optimization from PR #22450 Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * Reorder cli output * fix: follow getCalendar pattern in getAnalytics and maintain alphabetical order in turbo.json - Remove unnecessary object wrapping in getAnalytics.ts to match getCalendar.ts pattern - Move NEXT_PUBLIC_IS_E2E to correct alphabetical position in turbo.json globalEnv - Address PR feedback from keithwillcode Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * chore: update yarn.lock after analytics optimization changes Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* perf: implement quick app store loading optimizations - Add conditional app store imports in videoClient and handlePayment - Implement lazy calendar manager pattern in CalendarManager - Enhance createCachedImport with better concurrency handling - Create calendar-only registry for common calendar operations - Add performance instrumentation for debugging These optimizations reduce initial app store loading time by avoiding module-level imports and creating smaller, focused registries for calendar operations. Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * fix: revert getCalendar to use appStore for proper test mocking - Reverted getCalendar.ts to use main appStore instead of calendarStore - This ensures test mocking system works properly with existing appStoreMock - Fixes unit test failures where Google Calendar references were getting null values - All collective scheduling tests now pass Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * perf: implement CalendarServiceMap for optimized calendar loading - Add CalendarServiceMap generation following CrmServiceMap pattern - Update getCalendar.ts to use generated calendar service map - Remove manual calendar-registry.ts in favor of auto-generated approach - Reduces calendar initialization from loading 48+ apps to ~10 calendar apps Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * fix: update test mocks for CalendarServiceMap compatibility - Add missing SelectedCalendar fields (createdAt, updatedAt, lastErrorAt, watchAttempts, etc.) - Fix CredentialPayload type errors by adding user.email and delegationCredentialId - Mock CalendarServiceMap to use vi.importActual for real calendar services - Ensure calendar service tests work with new lazy loading approach Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * fix: resolve CalendarServiceMap test compatibility issues - Update getCalendarsEvents.test.ts mocks to work with CalendarServiceMap dynamic imports - Add missing SelectedCalendar fields (createdAt, updatedAt, lastErrorAt, watchAttempts, etc.) - Fix CredentialPayload type errors by adding user.email and delegationCredentialId - Use type assertion in getCalendar.ts to resolve credential type conflicts - Ensure calendar service tests work with new lazy loading approach Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * fix: update test mocking for CalendarServiceMap compatibility - Add comprehensive vi.mock for calendar.services.generated in delegation-credential tests - Mock GoogleCalendarService and Office365CalendarService with proper return values - Update all test files to use await with mockCalendarToHaveNoBusySlots - Ensure calendar events return expected meetingId, meetingPassword, meetingUrl values - Fix async/await compatibility issues in booking scenario test utilities Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * fix: resolve TypeScript errors in CalendarServiceMap mocking - Extract CalendarServiceMap promise to variable to fix 'always true' condition - Ensure vi.mocked is called on Promise type for proper mockResolvedValue access - Add await keywords to calendar mock calls in test files - Maintain existing functionality while making code type-safe Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * Remove Exchange 2013 and 2016 * Mock Exchange in all tests * Fix tests * Remove exchange 2013 and 2016 from app store index * Fix merge error * Await when getting calendar service * Fix selectedSlot test * Add missing variable * Update openapi.json * Updated CalendarService imports * try again * WIP migrate calendar apps to ES6 * Revert "WIP migrate calendar apps to ES6" This reverts commit 15bf2c8. * Revert changes back to e239910 This reverts all calendar service changes that were causing circular dependency issues during builds and E2E tests. * Remove circular dependency for location constants * Update yarn.lock with removed package * Add empty map when running E2E * Type fies * Fix merge conflict * Remove logging statements * Throw error and reset state if failing to load app * Revert "Remove Exchange 2013 and 2016" This reverts commit fedaf63. * Re-introduce exchange{2013,2016} Revert the removal in app-store/index.ts also. * Trying to fix tests --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Joe Au-Yeung <j.auyeung419@gmail.com> Co-authored-by: Joe Au-Yeung <65426560+joeauyeung@users.noreply.github.com> Co-authored-by: Alex van Andel <me@alexvanandel.com>
- Remove obsolete appStoreMock line from bookingScenario.ts since handlePayment now uses PaymentServiceMap - Update setupVitest.ts to import prismaMock from correct PrismockClient instance - Add PaymentServiceMap mock following PR #22450 pattern for calendar services - Ensure MockPaymentService uses consistent externalId across test files - Fix webhook handler to return 200 status by ensuring payment records are found correctly Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
…23408) * perf: optimize payment app imports to avoid loading entire app store - Add PaymentServiceMap generation to app-store-cli build process - Generate payment.services.generated.ts with lazy imports for 6 payment services - Update handlePayment.ts, deletePayment.ts, handlePaymentRefund.ts to use PaymentServiceMap - Update getConnectedApps.ts and tRPC payment routers to use PaymentServiceMap - Follow same pattern as analytics optimization in PR #23372 - Reduces bundle size by avoiding import of 100+ apps when only payment functionality needed Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * Update build.ts * fix: update payment service test mocking to work with PaymentServiceMap - Remove obsolete appStoreMock line from bookingScenario.ts since handlePayment now uses PaymentServiceMap - Update setupVitest.ts to import prismaMock from correct PrismockClient instance - Add PaymentServiceMap mock following PR #22450 pattern for calendar services - Ensure MockPaymentService uses consistent externalId across test files - Fix webhook handler to return 200 status by ensuring payment records are found correctly Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * fix: revert prismaMock import to avoid interfering with other tests' vi.spyOn() calls - Remove global prismaMock import from setupVitest.ts that was causing 'is not a spy' errors - Update MockPaymentService to import prismaMock locally to maintain payment test functionality - Fixes organization and outOfOffice tests while preserving payment service optimization Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * fix: remove E2E conditional check from payment services map generation - Payment services map now always includes all payment apps regardless of E2E environment - Ensures payment functionality is consistently available across all environments - Addresses CI failures caused by conditional payment service loading Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * refactor: use direct PaymentService imports instead of .lib structure - Update app-store-cli to import directly from lib/PaymentService.ts files - Modify all payment handlers to access PaymentService directly - Update test mocks to match new direct import structure - Remove .lib property access pattern across payment system - Maintain backward compatibility while improving import efficiency Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * fix: revert chargeCard booking.id parameter additions - Remove booking.id parameter from chargeCard calls in chargeCard.handler.ts and payments.tsx - Addresses GitHub feedback to investigate chargeCard signature changes in separate PR - Keeps all other direct PaymentService import refactor changes intact Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…gacy fallback - Move non-calendar map generation (payment, CRM, video) into packages/app-store-cli build/watch - Ensure maps regenerate on both `yarn build` and during `yarn dev` via CLI watcher - Update videoClient.ts to use VIDEO_ADAPTERS with lazy legacy fallback - Update handlePayment.ts to use PAYMENT_APPS with lazy legacy fallback - Remove eager @calcom/app-store imports (legacy loaded only if needed) Follows naming convention from calcom#22450 (calendar handled separately).
3 participants
What does this PR do?
This PR optimizes the Cal.com app-store calendar service imports by converting from dynamic imports to static imports, improving performance and consistency across the codebase.
Key Changes:
packages/app-store-cli/src/build.tsto generate static imports forCalendarServiceMapinstead of dynamic importsgetVariableNamefunction to properly handle forward slashes in import paths (regex:/[-.]/g→/[-.\/]/g)packages/app-store/_utils/getCalendar.tsto work with static imports by removing async/await and.defaultproperty accesspackages/app-store/calendar.services.generated.tswith the new static import structureLink to Devin run: https://app.devin.ai/sessions/8e0a5fbfba00485590fee40f95b84c11
Requested by: @joeauyeung
Visual Demo
Before (Dynamic Imports):
After (Static Imports):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Critical Test Areas:
Calendar Integration Testing:
Performance Verification:
Environment Setup:
Expected Behavior:
Test Data Requirements:
Checklist
Human Review Focus Areas
🔍 Critical Review Points:
getCalendardoesn't break any dependent code that expects promisesgetCalendarfunction🧪 Recommended Testing: