codeant-ai-for-open-source[bot] commented on code in PR #38346:
URL: https://github.com/apache/superset/pull/38346#discussion_r2873629901
##########
superset-frontend/src/core/commands/index.ts:
##########
@@ -50,15 +60,20 @@ const executeCommand: typeof commandsApi.executeCommand =
async <T>(
return callback(...args) as T;
};
-const getCommands: typeof commandsApi.getCommands = filterInternal => {
- const commands = Array.from(commandRegistry.keys());
- return Promise.resolve(
- filterInternal ? commands.filter(cmd => !cmd.startsWith('_')) : commands,
- );
+const getCommands: typeof commandsApi.getCommands = (): Command[] =>
+ Array.from(commandsMap.values());
+
+const getCommand: typeof commandsApi.getCommand = (
+ id: string,
+): Command | undefined => commandsMap.get(id);
+
+export const resetContributions = (): void => {
+ commandsMap.clear();
};
Review Comment:
**Suggestion:** The `resetContributions` function only clears the
`commandsMap` but leaves `commandRegistry` uncleared. This creates an
inconsistent state where command metadata is removed but callback functions
remain registered. After calling `resetContributions`, `getCommand` will return
undefined for existing commands, but `executeCommand` will still find and
execute the stale callbacks. Both maps should be cleared to properly reset the
command state. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Inconsistent state between command metadata and execution capability.
- ❌ Stale extension commands continue executing after reset/unload.
- ⚠️ Memory leaks accumulate orphaned callbacks in registry over extension
reloads.
```
</details>
```suggestion
commandRegistry.clear();
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Extension loads and registers commands via `registerCommand(command,
callback)` at
`superset-frontend/src/core/commands/index.ts:29`.
2. Registration populates both `commandsMap` (line 42) with command metadata
and
`commandRegistry` (line 44) with the callback function.
3. System calls `resetContributions()` at
`superset-frontend/src/core/commands/index.ts:70` to clear all command
contributions
(e.g., during extension unloading/reloading lifecycle).
4. `resetContributions` executes `commandsMap.clear()` (line 71) but omits
`commandRegistry.clear()`, leaving all callback functions registered.
5. Query `getCommand(id)` at line 67 returns `undefined` because metadata
was cleared, but
`executeCommand(id)` at line 52-60 still retrieves and executes the stale
callback from
`commandRegistry`, creating inconsistent state where commands appear
unregistered but
still execute.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/core/commands/index.ts
**Line:** 72:72
**Comment:**
*Logic Error: The `resetContributions` function only clears the
`commandsMap` but leaves `commandRegistry` uncleared. This creates an
inconsistent state where command metadata is removed but callback functions
remain registered. After calling `resetContributions`, `getCommand` will return
undefined for existing commands, but `executeCommand` will still find and
execute the stale callbacks. Both maps should be cleared to properly reset the
command state.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38346&comment_hash=df44f68eecb0987249a11c278c8b7cbce3887c2f910d02bf614971e8642b5470&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38346&comment_hash=df44f68eecb0987249a11c278c8b7cbce3887c2f910d02bf614971e8642b5470&reaction=dislike'>👎</a>
##########
superset-frontend/src/core/views/index.ts:
##########
@@ -0,0 +1,82 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * @fileoverview Standalone views registry implementation.
+ *
+ * Stores view metadata and providers as module-level state.
+ * Extensions register views as side effects at import time.
+ */
+
+import { ReactElement } from 'react';
+import type { views as viewsApi } from '@apache-superset/core';
+import { getExtensionsContextValue } from
'src/extensions/ExtensionsContextUtils';
+import { Disposable } from '../models';
+
+type View = viewsApi.View;
+
+const viewRegistry: Map<
+ string,
+ { view: View; location: string; provider: () => ReactElement }
+> = new Map();
+
+const locationIndex: Map<string, Set<string>> = new Map();
+
+const registerView: typeof viewsApi.registerView = (
+ view: View,
+ location: string,
+ provider: () => ReactElement,
+): Disposable => {
+ const { id } = view;
+
+ viewRegistry.set(id, { view, location, provider });
Review Comment:
**Suggestion:** Registering a view with an ID that already exists silently
overwrites the previous entry without disposing the old disposable or
unregistering the old provider. This causes memory leaks and state corruption
where the old disposable cleanup logic may accidentally clean up the new
registration. Add a check to prevent re-registration of existing view IDs or
properly dispose the old registration before overwriting. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Memory leak in ExtensionsContext from orphaned view providers.
- ⚠️ State inconsistency between viewRegistry and ExtensionsContext.
- ❌ Potential conflicts when multiple extensions accidentally use same view
ID.
```
</details>
```suggestion
if (viewRegistry.has(id)) {
throw new Error(`View with id '${id}' is already registered`);
}
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Extension A loads and calls `views.registerView({id: 'my-view'},
'location1',
providerA)` at `superset-frontend/src/core/views/index.ts:41-66`.
2. Function registers view in `viewRegistry` Map at line 48 and calls
`ctx.registerViewProvider('my-view', providerA)` at line 55.
3. Extension B (or a reload of Extension A) calls `views.registerView({id:
'my-view'},
'location2', providerB)` with identical view ID 'my-view'.
4. Second invocation overwrites the Map entry at line 48 without disposing
the previous
Disposable or unregistering the previous provider.
5. The first registration's Disposable object (lines 57-65) is orphaned and
never invoked,
leaving `providerA` permanently registered in ExtensionsContext via
`registerViewProvider`
with no way to unregister it.
6. Memory leak occurs in ExtensionsContext; viewRegistry and
ExtensionsContext become
inconsistent (registry has new provider, context has both).
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/core/views/index.ts
**Line:** 48:48
**Comment:**
*Possible Bug: Registering a view with an ID that already exists
silently overwrites the previous entry without disposing the old disposable or
unregistering the old provider. This causes memory leaks and state corruption
where the old disposable cleanup logic may accidentally clean up the new
registration. Add a check to prevent re-registration of existing view IDs or
properly dispose the old registration before overwriting.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38346&comment_hash=78c97c4888cc2f0b4c605733c44dfdf27bef8617e0be9770d7d173ad39f935f2&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38346&comment_hash=78c97c4888cc2f0b4c605733c44dfdf27bef8617e0be9770d7d173ad39f935f2&reaction=dislike'>👎</a>
##########
superset-frontend/src/components/ViewListExtension/index.tsx:
##########
@@ -16,29 +16,23 @@
* specific language governing permissions and limitations
* under the License.
*/
-import ExtensionsManager from 'src/extensions/ExtensionsManager';
+import { ensureIsArray } from '@superset-ui/core';
+import { views } from 'src/core';
import { useExtensionsContext } from 'src/extensions/ExtensionsContext';
export interface ViewListExtensionProps {
viewId: string;
}
const ViewListExtension = ({ viewId }: ViewListExtensionProps) => {
- const maybeContributions =
- ExtensionsManager.getInstance().getViewContributions(viewId);
- const contributions = Array.isArray(maybeContributions)
- ? maybeContributions
- : [];
+ const viewItems = ensureIsArray(views.getViews(viewId));
const { getView } = useExtensionsContext();
return (
<>
- {contributions
- .filter(
- contribution =>
- contribution && typeof contribution.id !== 'undefined',
- )
- .map(contribution => getView(contribution.id))}
+ {viewItems
+ .filter(view => view && typeof view.id !== 'undefined')
+ .map(view => getView(view.id))}
Review Comment:
**Suggestion:** Missing `key` prop when rendering list items in React. The
`.map()` call renders elements without providing a unique key prop, which
causes React reconciliation warnings and potential rendering issues. Each
element in a list should have a unique `key` prop (such as `view.id`) to help
React identify which items have changed. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ React console warnings during development for multi-view locations.
- ⚠️ Inefficient re-rendering when view lists reorder or update.
- ⚠️ Potential loss of component state if interactive views reorder.
```
</details>
```suggestion
.filter(view => view && typeof view.id !== 'undefined')
.map(view => (
<React.Fragment key={view.id}>
{getView(view.id)}
</React.Fragment>
))}
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Extension A registers a view for location 'sqllab.editor' using
`views.registerView(viewConfig, 'sqllab.editor', provider)` as per the new
code-first API
described in the PR.
2. Extension B registers a second view for the same location 'sqllab.editor'.
3. Superset renders SQL Lab page which imports and mounts `<ViewListExtension
viewId="sqllab.editor" />` at
`src/components/ViewListExtension/index.tsx:27`.
4. `views.getViews('sqllab.editor')` at line 28 returns an array of 2 view
objects.
5. The `.map()` at lines 34-35 iterates over both views, calling
`getView(view.id)` for
each without providing a `key` prop.
6. React DevTools console displays warning: "Warning: Each child in a list
should have a
unique 'key' prop."
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/components/ViewListExtension/index.tsx
**Line:** 34:35
**Comment:**
*Logic Error: Missing `key` prop when rendering list items in React.
The `.map()` call renders elements without providing a unique key prop, which
causes React reconciliation warnings and potential rendering issues. Each
element in a list should have a unique `key` prop (such as `view.id`) to help
React identify which items have changed.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38346&comment_hash=ba941bb26becd73a1c4533fce84b10e60aee0ad1f0fa46bf90b226122109b2c1&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38346&comment_hash=ba941bb26becd73a1c4533fce84b10e60aee0ad1f0fa46bf90b226122109b2c1&reaction=dislike'>👎</a>
##########
superset-frontend/src/extensions/ExtensionsLoader.ts:
##########
@@ -0,0 +1,161 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { SupersetClient } from '@superset-ui/core';
+import { logging } from '@apache-superset/core';
+import type { core } from '@apache-superset/core';
+
+type Extension = core.Extension;
+
+/**
+ * Loads extension modules via webpack module federation.
+ *
+ * Extensions register their contributions (commands, views, menus, editors)
+ * as module-level side effects when their module is imported. The loader
+ * only handles fetching and importing the remote modules.
+ */
+class ExtensionsLoader {
+ private static instance: ExtensionsLoader;
+
+ private extensionIndex: Map<string, Extension> = new Map();
+
+ // eslint-disable-next-line no-useless-constructor
+ private constructor() {
+ // Private constructor for singleton pattern
+ }
+
+ /**
+ * Singleton instance getter.
+ * @returns The singleton instance of ExtensionsLoader.
+ */
+ public static getInstance(): ExtensionsLoader {
+ if (!ExtensionsLoader.instance) {
+ ExtensionsLoader.instance = new ExtensionsLoader();
+ }
+ return ExtensionsLoader.instance;
+ }
+
+ /**
+ * Initializes extensions by fetching the list from the API and loading each
one.
+ * @throws Error if initialization fails.
+ */
+ public async initializeExtensions(): Promise<void> {
+ const response = await SupersetClient.get({
+ endpoint: '/api/v1/extensions/',
+ });
+ const extensions: Extension[] = response.json.result;
+ await Promise.all(
+ extensions.map(async extension => {
+ await this.initializeExtension(extension);
+ }),
+ );
+ }
+
+ /**
+ * Initializes a single extension.
+ * If the extension has a remote entry, loads the module (which triggers
+ * side-effect registrations for commands, views, menus, and editors).
+ * @param extension The extension to initialize.
+ */
+ public async initializeExtension(extension: Extension) {
+ try {
+ if (extension.remoteEntry) {
+ await this.loadModule(extension);
+ }
+ this.extensionIndex.set(extension.id, extension);
+ } catch (error) {
+ logging.error(
+ `Failed to initialize extension ${extension.name}\n`,
+ error,
+ );
+ }
+ }
+
+ /**
+ * Loads a single extension module via webpack module federation.
+ * The module's top-level side effects fire contribution registrations.
+ * @param extension The extension to load.
+ */
+ private async loadModule(extension: Extension): Promise<void> {
+ const { remoteEntry, id } = extension;
+
+ // Load the remote entry script
+ await new Promise<void>((resolve, reject) => {
+ const element = document.createElement('script');
+ element.src = remoteEntry;
+ element.type = 'text/javascript';
+ element.async = true;
+ element.onload = () => resolve();
+ element.onerror = (
+ event: Event | string,
+ source?: string,
+ lineno?: number,
+ colno?: number,
+ error?: Error,
+ ) => {
+ const errorDetails = [];
+ if (source) errorDetails.push(`source: ${source}`);
+ if (lineno !== undefined) errorDetails.push(`line: ${lineno}`);
+ if (colno !== undefined) errorDetails.push(`column: ${colno}`);
+ if (error?.message) errorDetails.push(`error: ${error.message}`);
+ if (typeof event === 'string') errorDetails.push(`event: ${event}`);
+
+ const detailsStr =
+ errorDetails.length > 0 ? `\n${errorDetails.join(', ')}` : '';
+ const errorMessage = `Failed to load remote entry:
${remoteEntry}${detailsStr}`;
+
+ return reject(new Error(errorMessage));
+ };
+
+ document.head.appendChild(element);
+ });
Review Comment:
**Suggestion:** Script element created for loading remote entries is
appended to document.head but never removed after loading (success or failure).
This causes memory leaks and DOM pollution, accumulating orphaned script tags
every time an extension is loaded. Wrap the loading logic in a try-finally
block to ensure the script element is removed from the DOM after the promise
settles. [resource leak]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Memory leak accumulated in browser DOM (orphaned script tags persist
for session)
- ⚠️ Developer experience degraded debugging DOM with many extension scripts
- ⚠️ Performance impact if extensions reloaded frequently (long-running SPAs)
```
</details>
```suggestion
private async loadModule(extension: Extension): Promise<void> {
const { remoteEntry, id } = extension;
// Load the remote entry script
const element = document.createElement('script');
element.src = remoteEntry;
element.type = 'text/javascript';
element.async = true;
try {
await new Promise<void>((resolve, reject) => {
element.onload = () => resolve();
element.onerror = (
event: Event | string,
source?: string,
lineno?: number,
colno?: number,
error?: Error,
) => {
const errorDetails = [];
if (source) errorDetails.push(`source: ${source}`);
if (lineno !== undefined) errorDetails.push(`line: ${lineno}`);
if (colno !== undefined) errorDetails.push(`column: ${colno}`);
if (error?.message) errorDetails.push(`error: ${error.message}`);
if (typeof event === 'string') errorDetails.push(`event: ${event}`);
const detailsStr =
errorDetails.length > 0 ? `\n${errorDetails.join(', ')}` : '';
const errorMessage = `Failed to load remote entry:
${remoteEntry}${detailsStr}`;
return reject(new Error(errorMessage));
};
document.head.appendChild(element);
});
} finally {
if (element.parentNode) {
element.parentNode.removeChild(element);
}
}
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Application bootstrap triggers
`ExtensionsLoader.getInstance().initializeExtensions()`
at application startup (entry point initialization).
2. `initializeExtensions()` at line 57 fetches extension list from
`/api/v1/extensions/`
endpoint via SupersetClient.
3. For each extension with `remoteEntry`, `loadModule()` at line 94 creates
a new
`<script>` element (line 99) and appends it to `document.head` (line 125)
inside Promise
executor.
4. Script `onload` handler resolves the Promise but no cleanup logic removes
the element
from DOM head afterward.
5. During development hot reload or if extensions are re-initialized (e.g.,
user logs out
and back in, triggering new initialization), additional script tags
accumulate in the DOM
without removal.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/extensions/ExtensionsLoader.ts
**Line:** 94:126
**Comment:**
*Resource Leak: Script element created for loading remote entries is
appended to document.head but never removed after loading (success or failure).
This causes memory leaks and DOM pollution, accumulating orphaned script tags
every time an extension is loaded. Wrap the loading logic in a try-finally
block to ensure the script element is removed from the DOM after the promise
settles.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38346&comment_hash=b18093b249d926be13d966882c5dda68a1f0167184bdab659e3dbbe7c1b076ea&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38346&comment_hash=b18093b249d926be13d966882c5dda68a1f0167184bdab659e3dbbe7c1b076ea&reaction=dislike'>👎</a>
##########
superset-frontend/src/extensions/ExtensionsLoader.ts:
##########
@@ -0,0 +1,161 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { SupersetClient } from '@superset-ui/core';
+import { logging } from '@apache-superset/core';
+import type { core } from '@apache-superset/core';
+
+type Extension = core.Extension;
+
+/**
+ * Loads extension modules via webpack module federation.
+ *
+ * Extensions register their contributions (commands, views, menus, editors)
+ * as module-level side effects when their module is imported. The loader
+ * only handles fetching and importing the remote modules.
+ */
+class ExtensionsLoader {
+ private static instance: ExtensionsLoader;
+
+ private extensionIndex: Map<string, Extension> = new Map();
+
+ // eslint-disable-next-line no-useless-constructor
+ private constructor() {
+ // Private constructor for singleton pattern
+ }
+
+ /**
+ * Singleton instance getter.
+ * @returns The singleton instance of ExtensionsLoader.
+ */
+ public static getInstance(): ExtensionsLoader {
+ if (!ExtensionsLoader.instance) {
+ ExtensionsLoader.instance = new ExtensionsLoader();
+ }
+ return ExtensionsLoader.instance;
+ }
+
+ /**
+ * Initializes extensions by fetching the list from the API and loading each
one.
+ * @throws Error if initialization fails.
+ */
+ public async initializeExtensions(): Promise<void> {
+ const response = await SupersetClient.get({
+ endpoint: '/api/v1/extensions/',
+ });
+ const extensions: Extension[] = response.json.result;
+ await Promise.all(
+ extensions.map(async extension => {
+ await this.initializeExtension(extension);
+ }),
+ );
+ }
+
+ /**
+ * Initializes a single extension.
+ * If the extension has a remote entry, loads the module (which triggers
+ * side-effect registrations for commands, views, menus, and editors).
+ * @param extension The extension to initialize.
+ */
+ public async initializeExtension(extension: Extension) {
+ try {
+ if (extension.remoteEntry) {
+ await this.loadModule(extension);
+ }
+ this.extensionIndex.set(extension.id, extension);
+ } catch (error) {
+ logging.error(
+ `Failed to initialize extension ${extension.name}\n`,
+ error,
+ );
Review Comment:
**Suggestion:** The `initializeExtension` method lacks concurrency
protection. If called multiple times for the same extension before the first
call completes (e.g., rapid re-renders or parallel initialization), it will
create multiple script tags and load the module multiple times, causing race
conditions and duplicate registrations. Add a loading promise cache to ensure
only one load operation per extension ID executes at a time. [race condition]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Duplicate extension registrations cause conflicts in contribution points
(menus, commands)
- ❌ Multiple identical remote entry scripts loaded (network waste, execution
overhead)
- ❌ Race conditions in global registration APIs corrupt extension state
```
</details>
```suggestion
private loadingPromises: Map<string, Promise<void>> = new Map();
public async initializeExtension(extension: Extension): Promise<void> {
// Prevent duplicate concurrent loading
if (this.extensionIndex.has(extension.id)) {
return;
}
if (this.loadingPromises.has(extension.id)) {
return this.loadingPromises.get(extension.id)!;
}
const loadPromise = (async () => {
try {
if (extension.remoteEntry) {
await this.loadModule(extension);
}
this.extensionIndex.set(extension.id, extension);
} catch (error) {
logging.error(
`Failed to initialize extension ${extension.name}\n`,
error,
);
throw error;
}
})();
this.loadingPromises.set(extension.id, loadPromise);
try {
await loadPromise;
} finally {
this.loadingPromises.delete(extension.id);
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. React application with StrictMode mounts component calling
`ExtensionsLoader.getInstance().initializeExtensions()` at line 57 during
component mount.
2. Before `Promise.all` at line 62 resolves, a re-render or navigation
triggers second
call to `initializeExtension()` at line 75 for same extension ID (race
condition window).
3. First call enters `initializeExtension()`, checks `extension.remoteEntry`
truthy at
line 77, and begins executing `loadModule()` at line 78 creating script tag.
4. Second concurrent call enters `initializeExtension()` at line 75, also
passes check at
line 77 because `extensionIndex` hasn't been set yet (first call still in
progress), and
begins second `loadModule()` execution.
5. Both calls create duplicate script elements loading same extension twice,
causing
duplicate registrations of menus/commands and race conditions in
contribution APIs.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/extensions/ExtensionsLoader.ts
**Line:** 75:85
**Comment:**
*Race Condition: The `initializeExtension` method lacks concurrency
protection. If called multiple times for the same extension before the first
call completes (e.g., rapid re-renders or parallel initialization), it will
create multiple script tags and load the module multiple times, causing race
conditions and duplicate registrations. Add a loading promise cache to ensure
only one load operation per extension ID executes at a time.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38346&comment_hash=15bc07a9ee466e7b6f65d0621f30ee7f968af0850aa4c6a9be719f56a96045e6&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38346&comment_hash=15bc07a9ee466e7b6f65d0621f30ee7f968af0850aa4c6a9be719f56a96045e6&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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
