EnxDev commented on code in PR #40441:
URL: https://github.com/apache/superset/pull/40441#discussion_r3340973082
##########
superset-frontend/src/extensions/ExtensionsStartup.tsx:
##########
@@ -73,21 +113,25 @@ const ExtensionsStartup: React.FC<{ children?:
React.ReactNode }> = ({
authentication,
core,
commands,
+ dashboard,
+ dataset,
editors,
+ explore,
extensions,
menus,
+ navigation,
sqlLab,
views,
};
- const setup = async () => {
- if (isFeatureEnabled(FeatureFlag.EnableExtensions)) {
- await ExtensionsLoader.getInstance().initializeExtensions();
- }
- setInitialized(true);
- };
+ // Render the host immediately; extension bundles load in the background.
+ // ChatbotMount re-resolves reactively once the chatbot extension registers
+ // (via subscribeToLocation), so the bubble appears without blocking the
UI.
Review Comment:
subscribeToLocation was renamed to the registry-wide subscribeToRegistry /
getRegistryVersion (used by ChatbotMount via useSyncExternalStore); the comment
was just stale. Updated it to match.
##########
superset-frontend/src/extensions/ExtensionsLoader.ts:
##########
@@ -88,23 +94,40 @@ class ExtensionsLoader {
public async initializeExtension(extension: Extension) {
try {
if (extension.remoteEntry) {
- await this.loadModule(extension);
+ const disposables = await this.loadModule(extension);
+ this.extensionDisposables.set(extension.id, disposables);
}
this.extensionIndex.set(extension.id, extension);
} catch (error) {
logging.error(
`Failed to initialize extension ${extension.name}\n`,
error,
);
+ store.dispatch(
+ addDangerToast(t('Extension "%s" failed to load.', extension.name)),
+ );
+ }
+ }
+
+ /**
+ * Deactivates an extension by disposing all of its registered contributions
+ * and removing it from the index.
+ */
+ public deactivateExtension(id: string): void {
+ const disposables = this.extensionDisposables.get(id);
+ if (disposables) {
+ disposables.forEach(dispose => dispose());
+ this.extensionDisposables.delete(id);
}
+ this.extensionIndex.delete(id);
}
Review Comment:
Good catch; that was true, and it was a real limitation. Disposables were
only collected by the window.superset registrar-wrapping that's active
synchronously during module evaluation, so anything an extension registered
after an await (or in a timer/event callback) escaped tracking and leaked on
deactivation.
Fixed by introducing an activate(context) contract: the extension pushes
each Disposable onto context.subscriptions, which the host owns for the
extension's whole lifetime. Because identity is tied to the context object
rather than a synchronous time window, async registrations are tracked the same
as sync ones. The registrar-wrapping is retained as a fallback for legacy
side-effect-only extensions (still sync-only, and documented as such). Added a
test that registers after an await inside activate and asserts it's disposed on
deactivation.
--
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]
