You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "michael-s-molina (via GitHub)" <gi...@apache.org> on 2026/06/09 19:21:18 UTC

[PR] fix(extensions): load extensions async to avoid blocking initial page render [superset]

michael-s-molina opened a new pull request, #40915:
URL: https://github.com/apache/superset/pull/40915

   ### SUMMARY
   
   Extension loading was blocking the entire React render tree on every page load. `ExtensionsStartup` wrapped the full route tree and returned `null` until `initializeExtensions()` resolved.
   
   **Root cause:** `ExtensionsStartup` awaited extension initialization before rendering children, blocking all routes including pages that have no dependency on extensions (e.g. the welcome page).
   
   **Fix — two parts:**
   
   1. **`ExtensionsStartup`: remove blocking gate.**
      `window.superset` setup is synchronous. Extension loading is now fire-and-forget — children render immediately and extensions register in the background.
   
   2. **`views` and `menus` registries: make reactive.**
      The registries were plain module-level data structures (a `Map` and an array). Components reading from them on render would miss any extensions that registered after initial paint. Added a `listeners` set and `notify()` calls on every `registerView`/`registerMenuItem`, and exported `useViews(location)` / `useMenu(location)` hooks that subscribe to changes and trigger re-renders when extensions register. Updated all four consumers:
      - `StatusBar` → `useViews`
      - `SouthPane` → `useViews`
      - `AppLayout` → `useViews`
      - `PanelToolbar` → `useMenu`
   
      Note: `EditorHost` already used a reactive pattern (`onDidRegisterEditor` events) and required no changes.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   | Before | After |
   |--------|-------|
   | Page renders blank until all extensions are fetched and loaded (~1–3s) | Page renders immediately; extension contributions appear once loaded |
   
   ### TESTING INSTRUCTIONS
   
   1. Enable the `ENABLE_EXTENSIONS` feature flag and configure at least one extension with a `remoteEntry`.
   2. Navigate to the welcome page — confirm it renders immediately without delay.
   3. Navigate to SQL Lab — confirm the page renders immediately.
   4. Wait for extensions to finish loading — confirm extension-contributed panels (SouthPane tabs), status bar items, right sidebar panels, and toolbar menus appear after load without a page refresh.
   5. To reproduce the pre-fix regression, temporarily add a delay in `ExtensionsLoader.initializeExtensions()` before the API call and confirm extension contributions appear reactively after the delay.
   
   ### ADDITIONAL INFORMATION
   
   - [ ] Has associated issue:
   - [ ] Required feature flags: `ENABLE_EXTENSIONS`
   - [x] Changes UI
   - [ ] Includes DB Migration
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(extensions): load extensions async to avoid blocking initial page render [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40915:
URL: https://github.com/apache/superset/pull/40915#discussion_r3383302470


##########
superset-frontend/src/core/menus/index.ts:
##########
@@ -77,6 +92,14 @@ const getMenu: typeof menusApi.getMenu = (
   return result;
 };
 
+export const useMenu = (location: string): Menu | undefined =>
+  useSyncExternalStore(subscribe, () => {
+    if (!menuCache.has(location)) {
+      menuCache.set(location, getMenu(location));
+    }
+    return menuCache.get(location);
+  });

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Missing getServerSnapshot for SSR</b></div>
   <div id="fix">
   
   The `useSyncExternalStore(subscribe, getSnapshot)` call at line 96 is missing the required third argument `getServerSnapshot`. React 18's useSyncExternalStore throws in environments where server and client render different snapshots (e.g., Next.js, Remix). The hook will also re-run on every mount in SSR unless a stable server snapshot is provided. Pass `() => getMenu(location)` as the third argument to fix SSR hydration safety.
   </div>
   
   
   <details>
   <summary>
   <b>Code suggestion</b>
   </summary>
   <blockquote>Check the AI-generated fix before applying</blockquote>
   <div id="code">
   
   
   ```
    --- superset-frontend/src/core/menus/index.ts
    +++ superset-frontend/src/core/menus/index.ts
    @@ -93,7 +93,8 @@ export const getMenu: typeof menusApi.getMenu = (
     };
     export const useMenu = (location: string): Menu | undefined =>
       useSyncExternalStore(subscribe, () => {
         if (!menuCache.has(location)) {
           menuCache.set(location, getMenu(location));
         }
         return menuCache.get(location);
    -  });
    +  }, () => getMenu(location));
   ```
   
   </div>
   </details>
   
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #23a437</i></small>
   </div>
   
   ---
   Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
   - [ ] Yes, avoid them



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(extensions): load extensions async to avoid blocking initial page render [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #40915:
URL: https://github.com/apache/superset/pull/40915#issuecomment-4663329265

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40915?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 64.13%. Comparing base ([`00e3682`](https://app.codecov.io/gh/apache/superset/commit/00e3682aaf1abd1c02b70bf56e2d604e812e8c7e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`f0ce4e8`](https://app.codecov.io/gh/apache/superset/commit/f0ce4e82f8ec980a88732e5f50209d36f5c6bee4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@           Coverage Diff           @@
   ##           master   #40915   +/-   ##
   =======================================
     Coverage   64.12%   64.13%           
   =======================================
     Files        2654     2654           
     Lines      143715   143736   +21     
     Branches    33150    33150           
   =======================================
   + Hits        92160    92181   +21     
     Misses      49943    49943           
     Partials     1612     1612           
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/40915/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [javascript](https://app.codecov.io/gh/apache/superset/pull/40915/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `67.88% <100.00%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/40915?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(extensions): load extensions async to avoid blocking initial page render [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40915:
URL: https://github.com/apache/superset/pull/40915#issuecomment-4664767448

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **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 <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(extensions): load extensions async to avoid blocking initial page render [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40915:
URL: https://github.com/apache/superset/pull/40915#discussion_r3383290698


##########
superset-frontend/src/extensions/ExtensionsStartup.tsx:
##########
@@ -52,20 +52,12 @@ declare global {
 const ExtensionsStartup: React.FC<{ children?: React.ReactNode }> = ({
   children,
 }) => {
-  const [initialized, setInitialized] = useState(false);
-
   const userId = useSelector<RootState, number | undefined>(
     ({ user }) => user.userId,
   );
 
   useEffect(() => {
-    if (initialized) return;
-
-    if (!userId) {
-      // No user logged in — nothing to initialize
-      setInitialized(true);
-      return;
-    }
+    if (!userId) return;

Review Comment:
   **Suggestion:** The login guard uses a falsy check on a numeric identifier, so a valid `userId` value of `0` will be treated as unauthenticated and skip both global API setup and extension initialization. Use an explicit null/undefined check instead of a generic falsy check. [falsy zero check]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Extensions never initialize for Redux userId equal zero.
   - ⚠️ window.superset APIs unavailable for zero-id user sessions.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Use the existing Jest testing harness in
   `superset-frontend/src/extensions/ExtensionsStartup.test.tsx` (see
   `render(<ExtensionsStartup />, { useRedux: true, initialState: mockInitialState })` at
   lines 72–76) and define an `initialState` object similar to `mockInitialState` (lines
   35–37) but with `user.userId` set to `0` instead of `1`.
   
   2. Run the test so that `<ExtensionsStartup />` mounts with Redux state containing `user:
   { userId: 0 }`, which is wired into the store via `setupStore` in
   `superset-frontend/src/views/store.ts` (lines 161–185) and the `userReducer` that takes
   `bootstrapData.user` or the provided initial state (lines 84–92).
   
   3. When `ExtensionsStartup` renders (component defined in
   `superset-frontend/src/extensions/ExtensionsStartup.tsx` at lines 52–81), the selector
   `useSelector<RootState, number | undefined>(({ user }) => user.userId)` (lines 55–57)
   returns `0`, and the `useEffect` hook at lines 59–78 executes.
   
   4. Inside the effect, the guard `if (!userId) return;` at line 60 treats `0` as falsy and
   returns early, so the code that sets `window.superset = { ...supersetCore, authentication,
   core, commands, editors, extensions, menus, sqlLab, views }` (lines 62–73) and calls
   `ExtensionsLoader.getInstance().initializeExtensions()` when
   `FeatureFlag.EnableExtensions` is enabled (lines 75–76) is skipped; asserting in the test
   that `(window as any).superset` is defined or that `initializeExtensions` was called (as
   done in the existing tests at lines 82–107 and 121–139) will fail for this zero-valued
   user ID.
   ```
   </details>
   
   [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=62f0f23eb8404550a7b1ae0cd285edab&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=62f0f23eb8404550a7b1ae0cd285edab&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/extensions/ExtensionsStartup.tsx
   **Line:** 60:60
   **Comment:**
   	*Falsy Zero Check: The login guard uses a falsy check on a numeric identifier, so a valid `userId` value of `0` will be treated as unauthenticated and skip both global API setup and extension initialization. Use an explicit null/undefined check instead of a generic falsy check.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40915&comment_hash=56ef45e8ab2f962c50d59ee09cafad8c365d1801817308007733ec3b9720f88a&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40915&comment_hash=56ef45e8ab2f962c50d59ee09cafad8c365d1801817308007733ec3b9720f88a&reaction=dislike'>👎</a>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(extensions): load extensions async to avoid blocking initial page render [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40915:
URL: https://github.com/apache/superset/pull/40915#discussion_r3384345590


##########
superset-frontend/src/core/menus/index.ts:
##########
@@ -77,6 +92,18 @@ const getMenu: typeof menusApi.getMenu = (
   return result;
 };
 
+export const useMenu = (location: string): Menu | undefined =>
+  useSyncExternalStore(
+    subscribe,
+    () => {
+      if (!menuCache.has(location)) {
+        menuCache.set(location, getMenu(location));
+      }
+      return menuCache.get(location);
+    },
+    () => undefined,

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>useSyncExternalStore hydration mismatch</b></div>
   <div id="fix">
   
   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`.
   </div>
   
   
   <details>
   <summary>
   <b>Code suggestion</b>
   </summary>
   <blockquote>Check the AI-generated fix before applying</blockquote>
   <div id="code">
   
   
   ```
    --- 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,
    -  );
    +    },
    +  );
   ```
   
   </div>
   </details>
   
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #d1e15d</i></small>
   </div>
   
   ---
   Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
   - [ ] Yes, avoid them



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(extensions): load extensions async to avoid blocking initial page render [superset]

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #40915:
URL: https://github.com/apache/superset/pull/40915#issuecomment-4679999493

   @eschutho You might want to check your bot as the comment above has nothing to do with the changes in this PR.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(extensions): load extensions async to avoid blocking initial page render [superset]

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #40915:
URL: https://github.com/apache/superset/pull/40915#issuecomment-4673573512

   > Nice improvement. One question: should the corresponding editor functionality be refactored to be in line with these new hooks?
   
   @villebro Thank you so much for this comment! It was the trigger for a better design that I implemented in [e1ac0a4](https://github.com/apache/superset/pull/40915/commits/e1ac0a4285739ce2ad01ad58bc0ea030ee089e7a).
   
   To improve consistency, I added a `useEditor(language)` hook for editors (mirroring the `useViews`/`useMenu` hooks introduced in this PR) and exposed `onDidRegisterView`/`onDidUnregisterView` and `onDidRegisterMenuItem`/`onDidUnregisterMenuItem` events for views and menus (mirroring the `onDidRegisterEditor`/`onDidUnregisterEditor` events that already existed for editors). The `EditorHost` component was simplified as a result — the manual useEffect-based subscription loop with its mount race workaround and suppressed  lint warning was replaced by a clean `useSyncExternalStore` call, the same pattern now used uniformly across all three registries.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(extensions): load extensions async to avoid blocking initial page render [superset]

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina merged PR #40915:
URL: https://github.com/apache/superset/pull/40915


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(extensions): load extensions async to avoid blocking initial page render [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40915:
URL: https://github.com/apache/superset/pull/40915#issuecomment-4675082599

   **Bito Automatic Review Skipped – PR Already Merged**<br/><br/>Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.<br/>No action is needed if you didn't intend to review it. To get a review, you can type `/review` in a comment and save it


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(extensions): load extensions async to avoid blocking initial page render [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40915:
URL: https://github.com/apache/superset/pull/40915#issuecomment-4663237430

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(extensions): load extensions async to avoid blocking initial page render [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40915:
URL: https://github.com/apache/superset/pull/40915#issuecomment-4663292125

   <!-- Bito Reply -->
   The flagged issue is correct. Using `if (!userId) return;` treats a `userId` of `0` as falsy, causing the initialization logic to be skipped for users with that ID. To resolve this, you should use an explicit check for `null` or `undefined`.
   
   Here is the corrected implementation for `superset-frontend/src/extensions/ExtensionsStartup.tsx`:
   
   ```tsx
       useEffect(() => {
         if (userId === null || userId === undefined) return;
   
         // Provide the implementations for @apache-superset/core
   ```
   
   I 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**
   ```
   useEffect(() => {
         if (userId === null || userId === undefined) return;
   
         // Provide the implementations for @apache-superset/core
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org