fix(extensions): load extensions async to avoid blocking initial page render#40915
fix(extensions): load extensions async to avoid blocking initial page render#40915michael-s-molina wants to merge 2 commits into
Conversation
michael-s-molina
requested review from
eschutho,
geido,
kgabryje,
rusackas and
villebro
as code owners
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
|
The flagged issue is correct. Using Here is the corrected implementation for useEffect(() => {
if (userId === null || userId === undefined) return;
// Provide the implementations for @apache-superset/coreI have checked the available PR comments and diffs, and there are no other pending review comments to address. Would you like me to perform any other analysis on this PR? superset-frontend/src/extensions/ExtensionsStartup.tsx |
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #23a437
Actionable Suggestions - 1
-
superset-frontend/src/core/menus/index.ts - 1
- Missing getServerSnapshot for SSR · Line 95-101
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset-frontend/src/SqlLab/components/AppLayout/index.tsx - 1
- Inconsistent data access pattern · Line 25-25
Review Details
-
Files reviewed - 7 · Commit Range:
f0ce4e8..f0ce4e8- superset-frontend/src/SqlLab/components/AppLayout/index.tsx
- superset-frontend/src/SqlLab/components/SouthPane/index.tsx
- superset-frontend/src/SqlLab/components/StatusBar/index.tsx
- superset-frontend/src/components/PanelToolbar/index.tsx
- superset-frontend/src/core/menus/index.ts
- superset-frontend/src/core/views/index.ts
- superset-frontend/src/extensions/ExtensionsStartup.tsx
-
Files skipped - 0
-
Tools
- Eslint (Linter) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #40915 +/- ##
=======================================
Coverage 64.12% 64.13%
=======================================
Files 2654 2654
Lines 143715 143740 +25
Branches 33150 33150
=======================================
+ Hits 92160 92182 +22
- Misses 49943 49946 +3
Partials 1612 1612
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- Use `userId == null` instead of `!userId` to handle userId=0 correctly - Add getServerSnapshot to useSyncExternalStore in useMenu and useViews Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
villebro
left a comment
villebro
left a comment
There was a problem hiding this comment.
Nice improvement. One question: should the corresponding editor functionality be refactored to be in line with these new hooks?
There was a problem hiding this comment.
Code Review Agent Run #d1e15d
Actionable Suggestions - 1
-
superset-frontend/src/core/menus/index.ts - 1
- useSyncExternalStore hydration mismatch · Line 104-104
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset-frontend/src/core/views/index.ts - 1
- Critical hook argument order fix · Line 94-104
Review Details
-
Files reviewed - 3 · Commit Range:
f0ce4e8..a5a6f1e- superset-frontend/src/core/menus/index.ts
- superset-frontend/src/core/views/index.ts
- superset-frontend/src/extensions/ExtensionsStartup.tsx
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Eslint (Linter) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
| } | ||
| return menuCache.get(location); | ||
| }, | ||
| () => undefined, |
There was a problem hiding this comment.
The added () => undefined as getServerSnapshot causes a hydration mismatch: server returns undefined while the client getSnapshot returns a Menu object. Per React docs, omit this argument when no server value is meaningful, or return the same value as getSnapshot.
Code suggestion
Check the AI-generated fix before applying
--- a/superset-frontend/src/core/menus/index.ts
+++ b/superset-frontend/src/core/menus/index.ts
@@ -95,11 +95,10 @@ export const useMenu = (location: string): Menu | undefined =>
useSyncExternalStore(
subscribe,
() => {
if (!menuCache.has(location)) {
menuCache.set(location, getMenu(location));
}
return menuCache.get(location);
- },
- () => undefined,
- );
+ },
+ );
Code Review Run #d1e15d
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
2 participants
SUMMARY
Extension loading was blocking the entire React render tree on every page load.
ExtensionsStartupwrapped the full route tree and returnednulluntilinitializeExtensions()resolved.Root cause:
ExtensionsStartupawaited extension initialization before rendering children, blocking all routes including pages that have no dependency on extensions (e.g. the welcome page).Fix — two parts:
ExtensionsStartup: remove blocking gate.window.supersetsetup is synchronous. Extension loading is now fire-and-forget — children render immediately and extensions register in the background.viewsandmenusregistries: make reactive.The registries were plain module-level data structures (a
Mapand an array). Components reading from them on render would miss any extensions that registered after initial paint. Added alistenersset andnotify()calls on everyregisterView/registerMenuItem, and exporteduseViews(location)/useMenu(location)hooks that subscribe to changes and trigger re-renders when extensions register. Updated all four consumers:StatusBar→useViewsSouthPane→useViewsAppLayout→useViewsPanelToolbar→useMenuNote:
EditorHostalready used a reactive pattern (onDidRegisterEditorevents) and required no changes.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ENABLE_EXTENSIONSfeature flag and configure at least one extension with aremoteEntry.ExtensionsLoader.initializeExtensions()before the API call and confirm extension contributions appear reactively after the delay.ADDITIONAL INFORMATION
ENABLE_EXTENSIONS