Merge pull request #5551 from nickgros/SWC-7102

src/main/java/org/sagebionetworks/web/client/SynapseJSNIUtils.java

@@ -15,8 +15,6 @@ public interface SynapseJSNIUtils {
 
   public void highlightCodeBlocks();
 
-  void loadSummaryDetailsShim();
-
   public void loadTableSorters();
 
   public String getBaseFileHandleUrl();

src/main/java/org/sagebionetworks/web/client/SynapseJSNIUtilsImpl.java

@@ -46,21 +46,6 @@ public static native void _highlightCodeBlocks() /*-{
 		}
 	}-*/;
 
-  @Override
-  public void loadSummaryDetailsShim() {
-    _loadSummaryDetailsShim();
-  }
-
-  public static native void _loadSummaryDetailsShim() /*-{
-		try {
-			$wnd.jQuery('summary').each(function(i, e) {
-				$wnd.details_shim(e)
-			});
-		} catch (err) {
-			console.error(err);
-		}
-	}-*/;
-
   @Override
   public void loadTableSorters() {
     _tablesorter();

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;
 }

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

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,10 @@ 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();
   }
 
   /**

src/main/java/org/sagebionetworks/web/client/widget/entity/MarkdownWidget.java

@@ -112,7 +112,6 @@ public void loadHtml(String uniqueSuffix, String result) {
     if (result != null && !result.isEmpty()) {
       view.setEmptyVisible(false);
       view.setMarkdown(result);
-      synapseJSNIUtils.loadSummaryDetailsShim();
       loadMath(uniqueSuffix);
       loadWidgets(wikiKey, wikiVersionInView, uniqueSuffix);
       loadTableSorters();

src/test/java/org/sagebionetworks/web/unitclient/widget/entity/MarkdownWidgetTest.java

@@ -141,9 +141,6 @@ public void testConfigureSuccess() {
     // verify tablesorter applied
     verify(mockSynapseJSNIUtils).loadTableSorters();
 
-    // verify summary/details tag shim run
-    verify(mockSynapseJSNIUtils).loadSummaryDetailsShim();
-
     // verify loadMath
     verify(mockSynapseJSNIUtils).processMath(mockElementWrapper.getElement());