Fix #23104 Circular Dependencies #25372
Open
+31
−3
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR tackles the performance bounty regarding the massive bundle size and 10+ second initial load times caused by the App Store.
The Diagnosis:
Using
madgeto analyze the dependency graph, I identified a critical circular dependency chain (75+ cycles) where the genericInstallAppButtonwas statically importing the entire App Store catalog (apps.browser.generated.tsx).The chain looked like this:
InstallAppButton->InstallAppButtonWithoutPlanCheck->InstallAppButtonMap(The whole store) ->Alby(and others) ->AppCard->OmniInstallAppButton->InstallAppButton.The Fix:
I refactored
InstallAppButtonWithoutPlanCheck.tsxto switch from a synchronous static import to an asynchronous dynamic import usinguseEffect.InstallAppButtonMapis now lazy-loaded only when the component mounts, preventing the entire App Store from being bundled into the initial page load of the Dashboard/Settings.Note:
Due to hardware limitations (my machine cannot handle the 16GB RAM requirement for the Node process in this monorepo) and issues verifying via Codespaces, I was unable to run the full dev server or the test suite locally.
I coded this fix based on static analysis data and assistance from LLMs (Gemini) to ensure the React Hooks logic (
useState,useEffect) handles the async loading safely (includingisMountedchecks). I am requesting the maintainers or reviewers to please verify the runtime behavior and test suite on my behalf.Visual Demo (For contributors especially)
Before the fix (Analysis Log):
Running
npx madge --circular --extensions ts,tsx packages/app-storeresulted in 75 circular dependencies. The root cause was the generic install button pulling in every app:After the fix:
The specific cycle involving
InstallAppButtonandapps.browser.generated.tsxis broken. Themadgecount should drop significantly (ideally to 1 or 0 runtime cycles), and the initial JS bundle size for the Dashboard should be drastically reduced.Mandatory Tasks (DO NOT REMOVE)
(Unchecked: As noted above, I could not run the test suite locally due to hardware constraints. I rely on CI/CD and reviewer assistance here.)
How should this be tested?
Since I could not run the environment locally, please test the following steps:
npx madge --circular --extensions ts,tsx packages/app-store. Confirm that the circular dependency count has dropped and theInstallAppButtonloop is gone.InstallAppButton..next/server/chunksfolder or the network tab. The initial bundle size should be smaller.yarn workspace @calcom/app-store test.getByRolemight need to be updated toawait findByRoleto handle theuseEffecttiming.Checklist