From 85f6ca60f867fdfc69d41ff708c2373b8c6ed2b3 Mon Sep 17 00:00:00 2001
From: Andrew Couch <hi@andrewcou.ch>
Date: Wed, 1 Aug 2018 14:13:14 -0400
Subject: [PATCH] ui: convert all uses of Loading to render prop

Contributes to: #24011
Release note: None
---
 .../views/clusterviz/containers/map/index.tsx |  5 +-
 .../containers/map/nodeCanvasContainer.tsx    | 19 +++---
 .../containers/dataDistribution/index.tsx     | 15 ++---
 .../cluster/containers/nodeLogs/index.tsx     |  5 +-
 pkg/ui/src/views/jobs/index.tsx               | 13 ++--
 .../reports/containers/commandQueue/index.tsx |  5 +-
 .../reports/containers/localities/index.tsx   | 33 +++++-----
 .../reports/containers/network/index.tsx      | 13 ++--
 .../containers/problemRanges/index.tsx        | 13 ++--
 .../reports/containers/range/allocator.tsx    | 37 +++++------
 .../containers/range/connectionsTable.tsx     | 63 ++++++++++---------
 .../reports/containers/range/logTable.tsx     | 51 +++++++--------
 .../reports/containers/settings/index.tsx     | 13 ++--
 .../views/shared/components/loading/index.tsx | 22 ++-----
 .../src/views/statements/statementDetails.tsx |  7 +--
 15 files changed, 156 insertions(+), 158 deletions(-)

diff --git a/pkg/ui/ccl/src/views/clusterviz/containers/map/index.tsx b/pkg/ui/ccl/src/views/clusterviz/containers/map/index.tsx
index c92daf9dbd86..1353f161d3f4 100644
--- a/pkg/ui/ccl/src/views/clusterviz/containers/map/index.tsx
+++ b/pkg/ui/ccl/src/views/clusterviz/containers/map/index.tsx
@@ -81,9 +81,8 @@ class ClusterVisualization extends React.Component<ClusterVisualizationProps & R
           loading={!this.props.licenseDataExists}
           className="loading-image loading-image__spinner-left"
           image={spinner}
-        >
-          <NodeCanvasContent tiers={tiers} />
-        </Loading>
+          render={() => <NodeCanvasContent tiers={tiers} />}
+        />
       </div>
     );
   }
diff --git a/pkg/ui/ccl/src/views/clusterviz/containers/map/nodeCanvasContainer.tsx b/pkg/ui/ccl/src/views/clusterviz/containers/map/nodeCanvasContainer.tsx
index c3cd8cac3394..79e6404f9f8f 100644
--- a/pkg/ui/ccl/src/views/clusterviz/containers/map/nodeCanvasContainer.tsx
+++ b/pkg/ui/ccl/src/views/clusterviz/containers/map/nodeCanvasContainer.tsx
@@ -75,15 +75,16 @@ class NodeCanvasContainer extends React.Component<NodeCanvasContainerProps & Nod
         loading={!this.props.dataExists}
         className="loading-image loading-image__spinner-left"
         image={spinner}
-      >
-        <NodeCanvas
-          localityTree={currentLocality}
-          locationTree={this.props.locationTree}
-          tiers={this.props.tiers}
-          livenessStatuses={this.props.livenessStatuses}
-          livenesses={this.props.livenesses}
-        />
-      </Loading>
+        render={() => (
+          <NodeCanvas
+            localityTree={currentLocality}
+            locationTree={this.props.locationTree}
+            tiers={this.props.tiers}
+            livenessStatuses={this.props.livenessStatuses}
+            livenesses={this.props.livenesses}
+          />
+        )}
+      />
     );
   }
 }
diff --git a/pkg/ui/src/views/cluster/containers/dataDistribution/index.tsx b/pkg/ui/src/views/cluster/containers/dataDistribution/index.tsx
index 217b8e1e9a02..457ffc08f1c8 100644
--- a/pkg/ui/src/views/cluster/containers/dataDistribution/index.tsx
+++ b/pkg/ui/src/views/cluster/containers/dataDistribution/index.tsx
@@ -147,13 +147,14 @@ class DataDistributionPage extends React.Component<DataDistributionPageProps> {
             className="loading-image loading-image__spinner-left"
             loading={!this.props.dataDistribution || !this.props.localityTree}
             image={spinner}
-          >
-            <DataDistribution
-              localityTree={this.props.localityTree}
-              dataDistribution={this.props.dataDistribution}
-              sortedZoneConfigs={this.props.sortedZoneConfigs}
-            />
-          </Loading>
+            render={() => (
+              <DataDistribution
+                localityTree={this.props.localityTree}
+                dataDistribution={this.props.dataDistribution}
+                sortedZoneConfigs={this.props.sortedZoneConfigs}
+              />
+            )}
+          />
         </section>
       </div>
     );
diff --git a/pkg/ui/src/views/cluster/containers/nodeLogs/index.tsx b/pkg/ui/src/views/cluster/containers/nodeLogs/index.tsx
index 1681e75a3540..ea3ea55d4f9f 100644
--- a/pkg/ui/src/views/cluster/containers/nodeLogs/index.tsx
+++ b/pkg/ui/src/views/cluster/containers/nodeLogs/index.tsx
@@ -110,9 +110,8 @@ class Logs extends React.Component<LogProps & RouterState, {}> {
             loading={ !this.props.logs.data }
             className="loading-image loading-image__spinner-left"
             image={ spinner }
-          >
-            { content }
-          </Loading>
+            render={() => content}
+          />
         </section>
       </div>
     );
diff --git a/pkg/ui/src/views/jobs/index.tsx b/pkg/ui/src/views/jobs/index.tsx
index bdcce5af446e..7fd5edf57ead 100644
--- a/pkg/ui/src/views/jobs/index.tsx
+++ b/pkg/ui/src/views/jobs/index.tsx
@@ -199,7 +199,8 @@ class JobsTable extends React.Component<JobsTableProps, {}> {
     this.props.setShow(selected.value);
   }
 
-  renderTable(jobs: Job[]) {
+  renderTable = () => {
+    const jobs = this.props.jobs && this.props.jobs.length > 0 && this.props.jobs;
     if (_.isEmpty(jobs)) {
       return <div className="no-results"><h2>No Results</h2></div>;
     }
@@ -218,7 +219,6 @@ class JobsTable extends React.Component<JobsTableProps, {}> {
   }
 
   render() {
-    const data = this.props.jobs && this.props.jobs.length > 0 && this.props.jobs;
     return <div className="jobs-page">
       <Helmet>
         <title>Jobs</title>
@@ -263,9 +263,12 @@ class JobsTable extends React.Component<JobsTableProps, {}> {
           </PageConfigItem>
         </PageConfig>
       </div>
-      <Loading loading={_.isNil(this.props.jobs)} className="loading-image loading-image__spinner" image={spinner}>
-          { this.renderTable(data) }
-      </Loading>
+      <Loading
+        loading={_.isNil(this.props.jobs)}
+        className="loading-image loading-image__spinner"
+        image={spinner}
+        render={this.renderTable}
+      />
     </div>;
   }
 }
diff --git a/pkg/ui/src/views/reports/containers/commandQueue/index.tsx b/pkg/ui/src/views/reports/containers/commandQueue/index.tsx
index 49b14444eff9..70178a93c6aa 100644
--- a/pkg/ui/src/views/reports/containers/commandQueue/index.tsx
+++ b/pkg/ui/src/views/reports/containers/commandQueue/index.tsx
@@ -110,9 +110,8 @@ class CommandQueue extends React.Component<CommandQueueProps, {}> {
           loading={!this.props.commandQueue || this.props.commandQueue.inFlight}
           className="loading-image loading-image__spinner-left"
           image={spinner}
-        >
-          {this.renderReportBody()}
-        </Loading>
+          render={this.renderReportBody}
+        />
       </div>
     );
   }
diff --git a/pkg/ui/src/views/reports/containers/localities/index.tsx b/pkg/ui/src/views/reports/containers/localities/index.tsx
index 3b8e6cf3cd18..d738e38981e4 100644
--- a/pkg/ui/src/views/reports/containers/localities/index.tsx
+++ b/pkg/ui/src/views/reports/containers/localities/index.tsx
@@ -100,22 +100,23 @@ class Localities extends React.Component<LocalitiesProps, {}> {
           loading={ !this.props.localityStatus.data || !this.props.locationStatus.data }
           className="loading-image loading-image__spinner-left"
           image={ spinner }
-        >
-          <section className="section">
-            <table className="locality-table">
-              <thead>
-                <tr>
-                  <th>Localities</th>
-                  <th>Nodes</th>
-                  <th>Location</th>
-                </tr>
-              </thead>
-              <tbody>
-                { renderLocalityTree(this.props.locationTree, this.props.localityTree) }
-              </tbody>
-            </table>
-          </section>
-        </Loading>
+          render={() => (
+            <section className="section">
+              <table className="locality-table">
+                <thead>
+                  <tr>
+                    <th>Localities</th>
+                    <th>Nodes</th>
+                    <th>Location</th>
+                  </tr>
+                </thead>
+                <tbody>
+                  { renderLocalityTree(this.props.locationTree, this.props.localityTree) }
+                </tbody>
+              </table>
+            </section>
+          )}
+        />
       </div>
     );
   }
diff --git a/pkg/ui/src/views/reports/containers/network/index.tsx b/pkg/ui/src/views/reports/containers/network/index.tsx
index 4dff1d457445..93db8e628338 100644
--- a/pkg/ui/src/views/reports/containers/network/index.tsx
+++ b/pkg/ui/src/views/reports/containers/network/index.tsx
@@ -446,12 +446,13 @@ class Network extends React.Component<NetworkProps, {}> {
           loading={!contentAvailable(nodesSummary)}
           className="loading-image loading-image__spinner-left loading-image__spinner-left__padded"
           image={spinner}
-        >
-          <div>
-            <NodeFilterList nodeIDs={filters.nodeIDs} localityRegex={filters.localityRegex} />
-            {this.renderContent(nodesSummary, filters)}
-          </div>
-        </Loading>
+          render={() => (
+            <div>
+              <NodeFilterList nodeIDs={filters.nodeIDs} localityRegex={filters.localityRegex} />
+              {this.renderContent(nodesSummary, filters)}
+            </div>
+          )}
+        />
       </div>
     );
   }
diff --git a/pkg/ui/src/views/reports/containers/problemRanges/index.tsx b/pkg/ui/src/views/reports/containers/problemRanges/index.tsx
index cbe16404fb26..8972726fb6b1 100644
--- a/pkg/ui/src/views/reports/containers/problemRanges/index.tsx
+++ b/pkg/ui/src/views/reports/containers/problemRanges/index.tsx
@@ -188,12 +188,13 @@ class ProblemRanges extends React.Component<ProblemRangesProps, {}> {
           loading={isLoading(this.props.problemRanges)}
           className="loading-image loading-image__spinner-left loading-image__spinner-left__padded"
           image={spinner}
-        >
-          <div>
-            {this.renderReportBody()}
-            <ConnectionsTable problemRanges={this.props.problemRanges} />
-          </div>
-        </Loading>
+          render={() => (
+            <div>
+              {this.renderReportBody()}
+              <ConnectionsTable problemRanges={this.props.problemRanges} />
+            </div>
+          )}
+        />
       </div>
     );
   }
diff --git a/pkg/ui/src/views/reports/containers/range/allocator.tsx b/pkg/ui/src/views/reports/containers/range/allocator.tsx
index d2469a598300..f0f5bb2658e8 100644
--- a/pkg/ui/src/views/reports/containers/range/allocator.tsx
+++ b/pkg/ui/src/views/reports/containers/range/allocator.tsx
@@ -59,24 +59,25 @@ export default class AllocatorOutput extends React.Component<AllocatorOutputProp
           loading={!allocator || allocator.inFlight}
           className="loading-image loading-image__spinner-left"
           image={spinner}
-        >
-          <table className="allocator-table">
-            <tbody>
-              <tr className="allocator-table__row allocator-table__row--header">
-                <th className="allocator-table__cell allocator-table__cell--header">Timestamp</th>
-                <th className="allocator-table__cell allocator-table__cell--header">Message</th>
-              </tr>
-              {
-                _.map(allocator.data.dry_run.events, (event, key) => (
-                  <tr key={key} className="allocator-table__row">
-                    <td className="allocator-table__cell allocator-table__cell--date">{Print.Timestamp(event.time)}</td>
-                    <td className="allocator-table__cell">{event.message}</td>
-                  </tr>
-                ))
-              }
-            </tbody>
-          </table>
-        </Loading>
+          render={() => (
+            <table className="allocator-table">
+              <tbody>
+                <tr className="allocator-table__row allocator-table__row--header">
+                  <th className="allocator-table__cell allocator-table__cell--header">Timestamp</th>
+                  <th className="allocator-table__cell allocator-table__cell--header">Message</th>
+                </tr>
+                {
+                  _.map(allocator.data.dry_run.events, (event, key) => (
+                    <tr key={key} className="allocator-table__row">
+                      <td className="allocator-table__cell allocator-table__cell--date">{Print.Timestamp(event.time)}</td>
+                      <td className="allocator-table__cell">{event.message}</td>
+                    </tr>
+                  ))
+                }
+              </tbody>
+            </table>
+          )}
+        />
       </div>
     );
   }
diff --git a/pkg/ui/src/views/reports/containers/range/connectionsTable.tsx b/pkg/ui/src/views/reports/containers/range/connectionsTable.tsx
index 48613e90f36a..18d56444aa52 100644
--- a/pkg/ui/src/views/reports/containers/range/connectionsTable.tsx
+++ b/pkg/ui/src/views/reports/containers/range/connectionsTable.tsx
@@ -31,37 +31,38 @@ export default function ConnectionsTable(props: ConnectionsTableProps) {
         loading={!range || range.inFlight}
         className="loading-image loading-image__spinner-left"
         image={spinner}
-      >
-        <table className="connections-table">
-          <tbody>
-            <tr className="connections-table__row connections-table__row--header">
-              <th className="connections-table__cell connections-table__cell--header">Node</th>
-              <th className="connections-table__cell connections-table__cell--header">Valid</th>
-              <th className="connections-table__cell connections-table__cell--header">Replicas</th>
-              <th className="connections-table__cell connections-table__cell--header">Error</th>
-            </tr>
-            {
-              _.map(ids, id => {
-                const resp = range.data.responses_by_node_id[id];
-                const rowClassName = classNames(
-                  "connections-table__row",
-                  { "connections-table__row--warning": !resp.response || !_.isEmpty(resp.error_message) },
-                );
-                return (
-                  <tr key={id} className={rowClassName}>
-                    <td className="connections-table__cell">n{id}</td>
-                    <td className="connections-table__cell">
-                      {resp.response ? "ok" : "error"}
-                    </td>
-                    <td className="connections-table__cell">{resp.infos.length}</td>
-                    <td className="connections-table__cell">{resp.error_message}</td>
-                  </tr>
-                );
-              })
-            }
-          </tbody>
-        </table>
-      </Loading>
+        render={() => (
+          <table className="connections-table">
+            <tbody>
+              <tr className="connections-table__row connections-table__row--header">
+                <th className="connections-table__cell connections-table__cell--header">Node</th>
+                <th className="connections-table__cell connections-table__cell--header">Valid</th>
+                <th className="connections-table__cell connections-table__cell--header">Replicas</th>
+                <th className="connections-table__cell connections-table__cell--header">Error</th>
+              </tr>
+              {
+                _.map(ids, id => {
+                  const resp = range.data.responses_by_node_id[id];
+                  const rowClassName = classNames(
+                    "connections-table__row",
+                    { "connections-table__row--warning": !resp.response || !_.isEmpty(resp.error_message) },
+                  );
+                  return (
+                    <tr key={id} className={rowClassName}>
+                      <td className="connections-table__cell">n{id}</td>
+                      <td className="connections-table__cell">
+                        {resp.response ? "ok" : "error"}
+                      </td>
+                      <td className="connections-table__cell">{resp.infos.length}</td>
+                      <td className="connections-table__cell">{resp.error_message}</td>
+                    </tr>
+                  );
+                })
+              }
+            </tbody>
+          </table>
+        )}
+      />
     </div>
   );
 }
diff --git a/pkg/ui/src/views/reports/containers/range/logTable.tsx b/pkg/ui/src/views/reports/containers/range/logTable.tsx
index b9a7d68e2606..77dfa6275adb 100644
--- a/pkg/ui/src/views/reports/containers/range/logTable.tsx
+++ b/pkg/ui/src/views/reports/containers/range/logTable.tsx
@@ -103,32 +103,33 @@ export default class LogTable extends React.Component<LogTableProps, {}> {
           loading={!log || log.inFlight}
           className="loading-image loading-image__spinner-left"
           image={spinner}
-        >
-          <table className="log-table">
-            <tbody>
-              <tr className="log-table__row log-table__row--header">
-                <th className="log-table__cell log-table__cell--header">Timestamp</th>
-                <th className="log-table__cell log-table__cell--header">Store</th>
-                <th className="log-table__cell log-table__cell--header">Event Type</th>
-                <th className="log-table__cell log-table__cell--header">Range</th>
-                <th className="log-table__cell log-table__cell--header">Other Range</th>
-                <th className="log-table__cell log-table__cell--header">Info</th>
-              </tr>
-              {_.map(events, (event, key) => (
-                <tr key={key} className="log-table__row">
-                  <td className="log-table__cell log-table__cell--date">
-                    {Print.Timestamp(event.event.timestamp)}
-                  </td>
-                  <td className="log-table__cell">s{event.event.store_id}</td>
-                  <td className="log-table__cell">{printLogEventType(event.event.event_type)}</td>
-                  <td className="log-table__cell">{this.renderRangeID(event.event.range_id)}</td>
-                  <td className="log-table__cell">{this.renderRangeID(event.event.other_range_id)}</td>
-                  <td className="log-table__cell">{this.renderLogInfo(event.pretty_info)}</td>
+          render={() => (
+            <table className="log-table">
+              <tbody>
+                <tr className="log-table__row log-table__row--header">
+                  <th className="log-table__cell log-table__cell--header">Timestamp</th>
+                  <th className="log-table__cell log-table__cell--header">Store</th>
+                  <th className="log-table__cell log-table__cell--header">Event Type</th>
+                  <th className="log-table__cell log-table__cell--header">Range</th>
+                  <th className="log-table__cell log-table__cell--header">Other Range</th>
+                  <th className="log-table__cell log-table__cell--header">Info</th>
                 </tr>
-              ))}
-            </tbody>
-          </table>
-        </Loading>
+                {_.map(events, (event, key) => (
+                  <tr key={key} className="log-table__row">
+                    <td className="log-table__cell log-table__cell--date">
+                      {Print.Timestamp(event.event.timestamp)}
+                    </td>
+                    <td className="log-table__cell">s{event.event.store_id}</td>
+                    <td className="log-table__cell">{printLogEventType(event.event.event_type)}</td>
+                    <td className="log-table__cell">{this.renderRangeID(event.event.range_id)}</td>
+                    <td className="log-table__cell">{this.renderRangeID(event.event.other_range_id)}</td>
+                    <td className="log-table__cell">{this.renderLogInfo(event.pretty_info)}</td>
+                  </tr>
+                ))}
+              </tbody>
+            </table>
+          )}
+        />
       </div>
     );
   }
diff --git a/pkg/ui/src/views/reports/containers/settings/index.tsx b/pkg/ui/src/views/reports/containers/settings/index.tsx
index 35581ed44c38..58f584eeaefd 100644
--- a/pkg/ui/src/views/reports/containers/settings/index.tsx
+++ b/pkg/ui/src/views/reports/containers/settings/index.tsx
@@ -87,12 +87,13 @@ class Settings extends React.Component<SettingsProps, {}> {
           loading={!this.props.settings.data}
           className="loading-image loading-image__spinner-left loading-image__spinner-left__padded"
           image={spinner}
-        >
-          <div>
-            <p>Note that some settings have been redacted for security purposes.</p>
-            {this.renderTable()}
-          </div>
-        </Loading>
+          render={() => (
+            <div>
+              <p>Note that some settings have been redacted for security purposes.</p>
+              {this.renderTable()}
+            </div>
+          )}
+        />
       </div>
     );
   }
diff --git a/pkg/ui/src/views/shared/components/loading/index.tsx b/pkg/ui/src/views/shared/components/loading/index.tsx
index 7d7f7dbfd5b3..ccbfe2726a01 100644
--- a/pkg/ui/src/views/shared/components/loading/index.tsx
+++ b/pkg/ui/src/views/shared/components/loading/index.tsx
@@ -4,11 +4,7 @@ interface LoadingProps {
   loading: boolean;
   className: string;
   image: string;
-  // The render function should probably be the only API, but currently
-  // it will be used if it is there, and fall back to children if it is not.
-  // TODO(vilterp): migrate all usages of Loading to use render prop.
-  render?: () => JSX.Element;
-  children?: React.ReactNode;
+  render: () => React.ReactNode;
 }
 
 // *
@@ -22,15 +18,9 @@ export default function Loading(props: LoadingProps) {
   if (props.loading) {
     return <div className={props.className} style={image} />;
   }
-  if (props.render) {
-    return props.render();
-  }
-
-  // This throws an error if more than one child is passed.
-  // Unfortunately the error seems to get eaten by some try/catch
-  // above this, but leaving it here to at least signal intent.
-  // Also unfortunately it's unclear how to enforce this invariant
-  // with the type system, since the `children` argument matches
-  // both one node and multiple nodes.
-  return React.Children.only(props.children);
+  return (
+    <React.Fragment>
+      { props.render() }
+    </React.Fragment>
+  );
 }
diff --git a/pkg/ui/src/views/statements/statementDetails.tsx b/pkg/ui/src/views/statements/statementDetails.tsx
index aa8b8fd06f56..ca5546fe8f3d 100644
--- a/pkg/ui/src/views/statements/statementDetails.tsx
+++ b/pkg/ui/src/views/statements/statementDetails.tsx
@@ -145,15 +145,14 @@ class StatementDetails extends React.Component<StatementDetailsProps, StatementD
             loading={_.isNil(this.props.statement)}
             className="loading-image loading-image__spinner"
             image={spinner}
-          >
-            { this.renderContent() }
-          </Loading>
+            render={this.renderContent}
+          />
         </section>
       </div>
     );
   }
 
-  renderContent() {
+  renderContent = () => {
     if (!this.props.statement) {
       return null;
     }