SWC-7102, SWC-7131 - Fix bugs in ReactComponent widgets

Sage-Bionetworks · Oct 25, 2024 · 5b0ddf9 · 5b0ddf9
1 parent b6f872a
commit 5b0ddf9
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 56 deletions.
diff --git a/src/main/java/org/sagebionetworks/web/client/jsinterop/React.java b/src/main/java/org/sagebionetworks/web/client/jsinterop/React.java
@@ -10,10 +10,7 @@ public class React {
 
   public static native <
     T extends ReactComponentType<P>, P extends ReactComponentProps
-  > ReactElement<T, P> createElement(
-    ReactComponentType<P> componentType,
-    P props
-  );
+  > ReactElement<T, P> createElement(ReactComponentType<P> componentType);
 
   public static native <
     T extends ReactComponentType<P>, P extends ReactComponentProps
@@ -83,4 +80,6 @@ public static native ReactElement cloneElement(
     ReactComponentProps props,
     ReactElement... children
   );
+
+  public static ReactComponentType<EmptyProps> Fragment;
 }
diff --git a/src/main/java/org/sagebionetworks/web/client/widget/ReactComponent.java b/src/main/java/org/sagebionetworks/web/client/widget/ReactComponent.java
@@ -7,6 +7,7 @@
 import com.google.gwt.event.shared.HandlerRegistration;
 import com.google.gwt.user.client.Timer;
 import com.google.gwt.user.client.ui.FlowPanel;
+import org.sagebionetworks.web.client.jsinterop.React;
 import org.sagebionetworks.web.client.jsinterop.ReactDOM;
 import org.sagebionetworks.web.client.jsinterop.ReactDOMRoot;
 import org.sagebionetworks.web.client.jsinterop.ReactElement;
@@ -33,12 +34,30 @@ private void createRoot() {
     }
   }
 
-  public void render(ReactElement<?, ?> reactElement) {
-    this.reactElement = reactElement;
+  /**
+   * Asynchronously (in the task queue, via setTimeout) unmounts the root and sets it to null.
+   */
+  private void destroyRoot() {
+    // React itself may have fired this method in its render cycle. If that's the case, we cannot unmount synchronously.
+    // We can asynchronously schedule unmounting the root to allow React to finish the current render cycle.
+    // https://github.com/facebook/react/issues/25675
+    Timer t = new Timer() {
+      @Override
+      public void run() {
+        if (root != null) {
+          root.unmount();
+          root = null;
+        }
+      }
+    };
+    t.schedule(0);
+  }
 
-    // This component may be a React child of another component. If so, we must rerender the ancestor component(s) so
-    // that they use the new ReactElement created in this render step.
-    // Asynchronously schedule creating a root in case React is still rendering and may unmount the current root
+  /**
+   * Asynchronously (in the task queue, via setTimeout) creates a root (if necessary) and renders the current reactElement.
+   */
+  private void createRootAndRender() {
+    // Asynchronously schedule createRoot and render to ensure any prequeued `destroyRoot` task completes first
     Timer t = new Timer() {
       @Override
       public void run() {
@@ -50,37 +69,32 @@ public void run() {
     t.schedule(0);
   }
 
+  public void render(ReactElement<?, ?> reactElement) {
+    this.reactElement = reactElement;
+    createRootAndRender();
+  }
+
   @Override
   protected void onLoad() {
     super.onLoad();
     createRoot();
+
     if (reactElement != null) {
       this.render(reactElement);
     }
   }
 
   @Override
   protected void onUnload() {
-    if (root != null) {
-      // Asynchronously schedule unmounting the root to allow React to finish the current render cycle.
-      // https://github.com/facebook/react/issues/25675
-      Timer t = new Timer() {
-        @Override
-        public void run() {
-          root.unmount();
-          root = null;
-        }
-      };
-      t.schedule(0);
-    }
+    destroyRoot();
     super.onUnload();
   }
 
   @Override
   public void clear() {
-    // clear doesn't typically call onUnload, but we want to for this element.
-    this.onUnload();
-    super.clear();
+    if (root != null) {
+      root.render(React.createElement(React.Fragment));
+    }
   }
 
   @Override

diff --git a/src/main/java/org/sagebionetworks/web/client/widget/ReactComponentV2.java b/src/main/java/org/sagebionetworks/web/client/widget/ReactComponentV2.java
@@ -92,11 +92,40 @@ private void createRoot() {
     }
   }
 
+  /**
+   * Asynchronously (in the task queue, via setTimeout) unmounts the root and sets it to null.
+   */
   private void destroyRoot() {
-    if (root != null) {
-      root.unmount();
-      root = null;
-    }
+    // React itself may have fired this method in its render cycle. If that's the case, we cannot unmount synchronously.
+    // We can asynchronously schedule unmounting the root to allow React to finish the current render cycle.
+    // https://github.com/facebook/react/issues/25675
+    Timer t = new Timer() {
+      @Override
+      public void run() {
+        if (root != null) {
+          root.unmount();
+          root = null;
+        }
+      }
+    };
+    t.schedule(0);
+  }
+
+  /**
+   * Asynchronously (in the task queue, via setTimeout) creates a root (if necessary) and renders the current reactElement.
+   */
+  private void createRootAndRender() {
+    // This component may be a React child of another component, so retrieve the root widget that renders this component tree.
+    ReactComponentV2<?, ?> componentToRender = getRootReactComponentWidget();
+    // Schedule creating a root and rendering. This will necessarily run after the `destroyRoot` completes, if it was invoked.
+    Timer t = new Timer() {
+      @Override
+      public void run() {
+        componentToRender.createRoot();
+        componentToRender.root.render(componentToRender.createReactElement());
+      }
+    };
+    t.schedule(0);
   }
 
   private void detachNonReactChildElements() {
@@ -181,24 +210,13 @@ public void render() {
     // This component will be rendered as a child of another React component, so destroy the root if one exists
     boolean shouldDestroyRoot = isRenderedAsReactComponentChild();
 
-    // This component may be a React child of another component, so retrieve the root widget that renders this component tree.
-    ReactComponentV2<?, ?> componentToRender = getRootReactComponentWidget();
-
-    // Asynchronously schedule root operations in case the component is in the middle of an asynchronous render cycle
-    // See https://stackoverflow.com/questions/73459382
-    Timer t = new Timer() {
-      @Override
-      public void run() {
-        if (shouldDestroyRoot) {
-          destroyRoot();
-        }
+    // Schedule destroying the root, if necessary
+    if (shouldDestroyRoot) {
+      destroyRoot();
+    }
 
-        // Create a fresh ReactElement tree and render it
-        componentToRender.createRoot();
-        componentToRender.root.render(componentToRender.createReactElement());
-      }
-    };
-    t.schedule(0);
+    // Schedule rendering
+    createRootAndRender();
   }
 
   @Override
@@ -216,20 +234,12 @@ protected void onLoad() {
 
   @Override
   protected void onUnload() {
-    super.onUnload();
-
     // Detach any non-React descendants that were injected into the component tree
     detachNonReactChildElements();
 
-    // Asynchronously schedule unmounting the root to allow React to finish the current render cycle.
-    // https://github.com/facebook/react/issues/25675
-    Timer t = new Timer() {
-      @Override
-      public void run() {
-        destroyRoot();
-      }
-    };
-    t.schedule(0);
+    destroyRoot();
+
+    super.onUnload();
   }
 
   /**