-
Notifications
You must be signed in to change notification settings - Fork 12k
fix:optimized data loading in locations.ts #24271
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -255,6 +255,7 @@ function generateFiles() { | |
| importName: "metadata", | ||
| }, | ||
| ], | ||
| lazyImport: true, | ||
| }) | ||
| ); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,39 @@ | ||
| import type { AppMeta } from "@calcom/types/App"; | ||
|
|
||
| import { appStoreMetadata as rawAppStoreMetadata } from "./apps.metadata.generated"; | ||
| import { getNormalizedAppMetadata } from "./getNormalizedAppMetadata"; | ||
|
|
||
| type RawAppStoreMetaData = typeof rawAppStoreMetadata; | ||
| type AppStoreMetaData = { | ||
| [key in keyof RawAppStoreMetaData]: Omit<AppMeta, "dirName"> & { dirName: string }; | ||
| }; | ||
| // Create a cache for loaded metadata | ||
| const metadataCache = new Map<string, AppMeta>(); | ||
|
|
||
| export const appStoreMetadata = {} as AppStoreMetaData; | ||
| for (const [key, value] of Object.entries(rawAppStoreMetadata)) { | ||
| appStoreMetadata[key as keyof typeof appStoreMetadata] = getNormalizedAppMetadata(value); | ||
| // Async function to get metadata by dynamically importing | ||
| export async function getAppMetadata(dirName: string): Promise<AppMeta | null> { | ||
| if (metadataCache.has(dirName)) { | ||
| return metadataCache.get(dirName)!; | ||
| } | ||
|
|
||
| try { | ||
| // Try to import config.json first | ||
| const configModule = await import(`./${dirName}/config.json`); | ||
| const normalized = getNormalizedAppMetadata(configModule.default); | ||
| metadataCache.set(dirName, normalized); | ||
| return normalized; | ||
| } catch { | ||
| try { | ||
| // Fallback to _metadata.ts | ||
| const metadataModule = await import(`./${dirName}/_metadata`); | ||
| const normalized = getNormalizedAppMetadata(metadataModule.metadata); | ||
| metadataCache.set(dirName, normalized); | ||
| return normalized; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Keep the old synchronous interface for backward compatibility | ||
| // But make it throw an error to encourage migration to async | ||
| export const appStoreMetadata = new Proxy({} as any, { | ||
| get(target, prop: string) { | ||
| throw new Error(`appStoreMetadata is no longer synchronous. Use getAppMetadata('${prop}') instead.`); | ||
|
Check failure on line 37 in packages/app-store/appStoreMetaData.ts
|
||
| }, | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| // If you import this file on any app it should produce circular dependency | ||
| // import appStore from "./index"; | ||
| import { appStoreMetadata } from "@calcom/app-store/appStoreMetaData"; | ||
| import { getAppMetadata } from "@calcom/app-store/appStoreMetaData"; | ||
| import type { EventLocationType } from "@calcom/app-store/locations"; | ||
| import logger from "@calcom/lib/logger"; | ||
| import { getPiiFreeCredential } from "@calcom/lib/piiFreeData"; | ||
|
|
@@ -18,27 +18,33 @@ export type LocationOption = { | |
| disabled?: boolean; | ||
| }; | ||
|
|
||
| const ALL_APPS_MAP = Object.keys(appStoreMetadata).reduce((store, key) => { | ||
| const metadata = appStoreMetadata[key as keyof typeof appStoreMetadata] as AppMeta; | ||
| // Cache for loaded apps | ||
| let ALL_APPS_CACHE: AppMeta[] | null = null; | ||
| let ALL_APPS_MAP_CACHE: Record<string, AppMeta> | null = null; | ||
|
|
||
| store[key] = metadata; | ||
| // Function to load all apps metadata | ||
| async function loadAllApps() { | ||
| if (ALL_APPS_CACHE && ALL_APPS_MAP_CACHE) { | ||
| return { apps: ALL_APPS_CACHE, appsMap: ALL_APPS_MAP_CACHE }; | ||
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| //@ts-ignore | ||
| delete store[key]["/*"]; | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| //@ts-ignore | ||
| delete store[key]["__createdUsingCli"]; | ||
| return store; | ||
| }, {} as Record<string, AppMeta>); | ||
| // For now, return empty arrays since we can't synchronously load all apps | ||
| // This needs to be changed to load apps from database or a static list | ||
| ALL_APPS_CACHE = []; | ||
| ALL_APPS_MAP_CACHE = {}; | ||
| return { apps: ALL_APPS_CACHE, appsMap: ALL_APPS_MAP_CACHE }; | ||
| } | ||
|
|
||
| export type CredentialDataWithTeamName = CredentialForCalendarService & { | ||
| team?: { | ||
| name: string; | ||
| } | null; | ||
| }; | ||
|
|
||
| export const ALL_APPS = Object.values(ALL_APPS_MAP); | ||
| // For backward compatibility, provide synchronous access | ||
| // But this will be empty until we implement proper loading | ||
| export const ALL_APPS: AppMeta[] = []; | ||
| const ALL_APPS_MAP: Record<string, AppMeta> = {}; | ||
|
Comment on lines
+31
to
+47
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking regression: ALL_APPS/slug lookups now always return empty
Also applies to: 139-143 🤖 Prompt for AI Agents |
||
|
|
||
| /** | ||
| * This should get all available apps to the user based on his saved | ||
|
|
@@ -131,7 +137,9 @@ export function getAppType(name: string): string { | |
| } | ||
|
|
||
| export function getAppFromSlug(slug: string | undefined): AppMeta | undefined { | ||
| return ALL_APPS.find((app) => app.slug === slug); | ||
| // For now, return undefined since we don't have preloaded data | ||
| // This needs to be implemented properly | ||
| return undefined; | ||
| } | ||
|
|
||
| export function getAppFromLocationValue(type: string): AppMeta | undefined { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking: lazy loader still relies on
appStoreMetadataProxybookerAppsMetaDatano longer exposes a concreteappStoreMetadata; in this PR it’s now a Proxy that throws on property access to prevent the old eager pattern. Destructuring it here trips that guard, we fall into the catch block, andlocationsFromAppsis permanently set to[]. Every downstream lookup (getLocationFromApp,LocationType, etc.) now loses all app-provided locations, breaking conferencing apps across the product. Please switch this loader over to the new lazy API (e.g. hydrate viagetAppMetadata/ the new registry) before caching so we still populate real metadata.