Skip to content

perf: optimize video adapter imports to avoid loading entire app store#23435

Merged
keithwillcode merged 7 commits intomainfrom
devin/video-adapters-refactor-1756421353
Aug 29, 2025
Merged

perf: optimize video adapter imports to avoid loading entire app store#23435
keithwillcode merged 7 commits intomainfrom
devin/video-adapters-refactor-1756421353

Conversation

@keithwillcode
Copy link
Contributor

@keithwillcode keithwillcode commented Aug 28, 2025 •
edited
Loading

What does this PR do?

This PR refactors video adapters to use a dedicated VideoApiAdapterMap instead of dynamically importing from the entire app store, following the same pattern established for calendar and payment services in PRs #22450, #23372, and #23408.

Key Changes:

  • Creates packages/app-store/video.adapters.generated.ts with a VideoApiAdapterMap containing all video adapters
  • Updates packages/lib/videoClient.ts to use the new map instead of dynamic app store imports
  • Refactors packages/trpc/server/routers/viewer/calVideo/getMeetingInformation.handler.ts to use the map
  • Updates build system in packages/app-store-cli/src/build.ts to generate the video services map
  • Updates test mocks in bookingScenario.ts to work with the new import structure

Motivation: This change improves performance by replacing dynamic imports with static imports and provides better type safety and consistency across the codebase.

Link to Devin run: https://app.devin.ai/sessions/179257529a2248ea95efc8258d8d3875
Requested by: @keithwillcode

How should this be tested?

Critical Test Cases:

  1. Zoom app name transformation - Verify zoom_video correctly transforms to zoomvideo
  2. Video meeting operations - Test create, update, delete video meetings across different adapters
  3. Error handling - Test behavior when video adapters are missing or fail to load
  4. E2E test environment - Verify E2E tests still work with the conditional empty map

Environment Setup:

  • Standard Cal.com development environment
  • No special environment variables needed

Test Commands:

# Run video-related unit tests
TZ=UTC yarn test packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts

# Run type checks
yarn type-check:ci --force

# Test video meeting functionality end-to-end with various video apps

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox. N/A - Internal refactoring with no API changes.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

Human Reviewer Checklist

⚠️ High Priority Items:

  • Zoom name parsing: Verify the zoom_video → zoomvideo transformation still works correctly in getVideoAdapters()
  • Video adapter coverage: Confirm all video adapters are properly included in the generated VideoApiAdapterMap
  • Test mock accuracy: Validate that the new Proxy-based mock registry in bookingScenario.ts accurately simulates the new import behavior
  • Error handling: Test scenarios where video adapters are missing or fail to load - ensure proper error messages

Standard Review Items:

  • Generated file looks correct and includes all expected video adapters
  • Import paths are consistent and follow established patterns
  • No breaking changes to existing video meeting functionality
  • E2E test environment handling works correctly (empty map for NEXT_PUBLIC_IS_E2E)

@devin-ai-integration @keithwillcode
perf: optimize video adapter imports to avoid loading entire app store
cca31e9 
- Creates VideoApiAdapterMap with lazy imports for 12 video services
- Updates getVideoAdapters function to use VideoApiAdapterMap instead of dynamic app store imports
- Preserves zoom app name parsing logic (zoom_video → zoomvideo)
- Follows same optimization pattern as calendar, analytics, and payment services
- Reduces bundle size by avoiding import of 100+ apps when only video functionality needed

Affected files:
- packages/app-store-cli/src/build.ts: Added video service generation logic
- packages/lib/videoClient.ts: Updated to use VideoApiAdapterMap
- packages/features/bookings/lib/handleCancelBooking.ts: Updated FAKE_DAILY_CREDENTIAL import
- packages/lib/EventManager.ts: Updated FAKE_DAILY_CREDENTIAL import
- packages/trpc/server/routers/viewer/calVideo/getMeetingInformation.handler.ts: Updated to use VideoApiAdapterMap
- apps/web/lib/video/[uid]/getServerSideProps.ts: Updated daily video function imports
- packages/app-store/video.services.generated.ts: Generated video adapter map with re-exports

Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
@devin-ai-integration
Copy link
Contributor

devin-ai-integration bot commented Aug 28, 2025

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@keithwillcode keithwillcode added core area: core, team members only foundation labels Aug 28, 2025
@devin-ai-integration @keithwillcode
fix: add missing re-exports to video.services.generated.ts
af2d916 
- Updates build.ts to include FAKE_DAILY_CREDENTIAL and other daily video function re-exports
- Fixes type errors in EventManager.ts and other files importing from video.services.generated
- Ensures video adapter refactoring maintains all existing functionality

Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
@vercel
Copy link

vercel bot commented Aug 28, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Aug 29, 2025 1:59am
cal-eu Ignored Ignored Aug 29, 2025 1:59am

@devin-ai-integration @keithwillcode
fix: update video adapter test mocks to work with VideoApiAdapterMap
aebfe48 
- Creates global mockVideoAdapterRegistry for dynamic video adapter mocks
- Uses Proxy in vi.mock for VideoApiAdapterMap to return registered mocks
- Updates mockVideoApp and mockErrorOnVideoMeetingCreation to register mocks
- Fixes unit test failures in booking scenario tests
- Ensures video meeting operations (createMeeting, updateMeeting, deleteMeeting) work correctly

Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
@keithwillcode keithwillcode self-assigned this Aug 29, 2025
@keithwillcode keithwillcode added this to the v5.7 milestone Aug 29, 2025
keithwillcode
keithwillcode commented Aug 29, 2025
View reviewed changes
@github-actions
Copy link
Contributor

github-actions bot commented Aug 29, 2025 •
edited
Loading

E2E results are ready!

@devin-ai-integration @keithwillcode
fix: remove re-exports from video.services.generated.ts and revert im…
2e17b8d 
…ports

- Remove re-export block from video.services.generated.ts as requested
- Revert imports back to pull directly from dailyvideo/lib/VideoApiAdapter
- Update build.ts to not generate the re-exports
- Maintains all existing functionality while addressing GitHub feedback

Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
keithwillcode
keithwillcode commented Aug 29, 2025
View reviewed changes
keithwillcode
keithwillcode commented Aug 29, 2025
View reviewed changes
@devin-ai-integration @keithwillcode
fix: rename video.services.generated.ts to video.adapters.generated.ts
b5240ae 
- Updates build.ts to generate video.adapters.generated.ts instead of video.services.generated.ts
- Updates all import statements to use new filename
- Removes unnecessary mock exports from bookingScenario.ts (FAKE_DAILY_CREDENTIAL, etc.)
- Addresses GitHub comments from @keithwillcode on PR #23435

The terminology change from 'services map' to 'adapters map' better reflects
the actual content (video adapters, not services) and maintains consistency
with the established refactoring pattern.

Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
keithwillcode
keithwillcode commented Aug 29, 2025
View reviewed changes
keithwillcode
keithwillcode commented Aug 29, 2025
View reviewed changes
@devin-ai-integration @keithwillcode
refactor: rename videoServices to videoAdapters and simplify return s…
abd6671 
…tatement

- Rename variable from videoServices to videoAdapters for consistency
- Remove unnecessary const variable and return directly in same line
- Addresses GitHub comments from @keithwillcode on PR #23435

Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
@keithwillcode keithwillcode mentioned this pull request Aug 29, 2025
Closed
@keithwillcode keithwillcode marked this pull request as ready for review August 29, 2025 01:27
@graphite-app graphite-app bot requested a review from a team August 29, 2025 01:28
@dosubot dosubot bot added app-store area: app store, apps, calendar integrations, google calendar, outlook, lark, apple calendar 💻 refactor labels Aug 29, 2025
@calcom calcom deleted a comment from coderabbitai bot Aug 29, 2025
@keithwillcode keithwillcode enabled auto-merge (squash) August 29, 2025 03:25
hbjORbj
hbjORbj approved these changes Aug 29, 2025
View reviewed changes
Copy link
Contributor

@hbjORbj hbjORbj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YESSSSS

@keithwillcode keithwillcode merged commit 191db51 into main Aug 29, 2025
65 of 67 checks passed
@keithwillcode keithwillcode deleted the devin/video-adapters-refactor-1756421353 branch August 29, 2025 05:21
@anglerfishlyy anglerfishlyy mentioned this pull request Aug 30, 2025
Closed
@keithwillcode keithwillcode mentioned this pull request Aug 30, 2025
Closed
3 tasks
@eleith eleith mentioned this pull request Sep 10, 2025
Closed
@michaeloboyle michaeloboyle mentioned this pull request Oct 25, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hbjORbj hbjORbj hbjORbj approved these changes

Assignees

@keithwillcode keithwillcode

Labels

app-store area: app store, apps, calendar integrations, google calendar, outlook, lark, apple calendar core area: core, team members only foundation ready-for-e2e 💻 refactor

Projects

None yet

Milestone

v5.7

Development

Successfully merging this pull request may close these issues.

2 participants

@keithwillcode @hbjORbj