From 610e46433c3089a89d3acfeb85fe15b4da783ade Mon Sep 17 00:00:00 2001
From: Akos Kitta <a.kitta@arduino.cc>
Date: Thu, 3 Aug 2023 15:15:36 +0200
Subject: [PATCH] fix: do not let the React dialogs dispose on close

To workaround the React-based widget lifecycle issue:
eclipse-theia/theia#12093.

The dialog constructor is called once, hence the default `toDispose`
listener will execute only once per app lifecycle. The default dialog
close behavior (from Theia) will dispose the widget. On dialog reopen,
the constructor won't be called again, as the dialog is injected as
a singleton, but the React component has been unmounted on dialog
dispose. Hence, they will be recreated. The `componentDidMount`
method will be called, but the `componentWillUnmount` is not called
anymore. The dialog is already disposed. It leads to a resource leak.

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
---
 .../src/browser/theia/dialogs/dialogs.tsx     | 53 +++++++------------
 1 file changed, 18 insertions(+), 35 deletions(-)

diff --git a/arduino-ide-extension/src/browser/theia/dialogs/dialogs.tsx b/arduino-ide-extension/src/browser/theia/dialogs/dialogs.tsx
index 20284b413..c354bfd77 100644
--- a/arduino-ide-extension/src/browser/theia/dialogs/dialogs.tsx
+++ b/arduino-ide-extension/src/browser/theia/dialogs/dialogs.tsx
@@ -3,14 +3,9 @@ import {
   DialogProps,
 } from '@theia/core/lib/browser/dialogs';
 import { ReactDialog as TheiaReactDialog } from '@theia/core/lib/browser/dialogs/react-dialog';
-import { codiconArray, Message } from '@theia/core/lib/browser/widgets/widget';
-import {
-  Disposable,
-  DisposableCollection,
-} from '@theia/core/lib/common/disposable';
+import { codiconArray } from '@theia/core/lib/browser/widgets/widget';
+import type { Message } from '@theia/core/shared/@phosphor/messaging';
 import { inject, injectable } from '@theia/core/shared/inversify';
-import * as React from '@theia/core/shared/react';
-import { createRoot } from '@theia/core/shared/react-dom/client';
 
 @injectable()
 export abstract class AbstractDialog<T> extends TheiaAbstractDialog<T> {
@@ -25,38 +20,26 @@ export abstract class AbstractDialog<T> extends TheiaAbstractDialog<T> {
 
 @injectable()
 export abstract class ReactDialog<T> extends TheiaReactDialog<T> {
-  protected override onUpdateRequest(msg: Message): void {
-    // This is tricky to bypass the default Theia code.
-    // Otherwise, there is a warning when opening the dialog for the second time.
-    // You are calling ReactDOMClient.createRoot() on a container that has already been passed to createRoot() before. Instead, call root.render() on the existing root instead if you want to update it.
-    const disposables = new DisposableCollection();
-    if (!this.isMounted) {
-      // toggle the `isMounted` logic for the time being of the super call so that the `createRoot` does not run
-      this.isMounted = true;
-      disposables.push(Disposable.create(() => (this.isMounted = false)));
-    }
+  private _isOnCloseRequestInProgress = false;
 
-    // Always unset the `contentNodeRoot` so there is no double update when calling super.
-    const restoreContentNodeRoot = this.contentNodeRoot;
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    (this.contentNodeRoot as any) = undefined;
-    disposables.push(
-      Disposable.create(() => (this.contentNodeRoot = restoreContentNodeRoot))
-    );
+  override dispose(): void {
+    // There is a bug in Theia, and the React component's `componentWillUnmount` will not be called, as the Theia widget is already disposed when closing and reopening a dialog.
+    // Widget lifecycle issue in Theia: https://github.com/eclipse-theia/theia/issues/12093
+    // Bogus react widget lifecycle management PR: https://github.com/eclipse-theia/theia/pull/11687
+    // Do not call super. Do not let the Phosphor widget to be disposed on dialog close.
+    if (this._isOnCloseRequestInProgress) {
+      // Do not let the widget dispose on close.
+      return;
+    }
+    super.dispose();
+  }
 
+  protected override onCloseRequest(message: Message): void {
+    this._isOnCloseRequestInProgress = true;
     try {
-      super.onUpdateRequest(msg);
+      super.onCloseRequest(message);
     } finally {
-      disposables.dispose();
-    }
-
-    // Use the patched rendering.
-    if (!this.isMounted) {
-      this.contentNodeRoot = createRoot(this.contentNode);
-      // Resetting the prop is missing from the Theia code.
-      // https://github.com/eclipse-theia/theia/blob/v1.31.1/packages/core/src/browser/dialogs/react-dialog.tsx#L41-L47
-      this.isMounted = true;
+      this._isOnCloseRequestInProgress = false;
     }
-    this.contentNodeRoot?.render(<>{this.render()}</>);
   }
 }