Merge cockroachdb#43846

43846: ui: Upgrade React version and react-dependent libraries r=dhartunian a=Koorosh - Has to merged after: cockroachdb#43685 - Depends on cockroachdb/yarn-vendored#13 Current PR has affected a lot of files but most of changes are almost the same and do not affect business logic. With current changes, react is upgraded to latest version (16.12.0) and following libraries are upgraded as well: react-redux, redux, redux-saga. These upgrades of major versions has required to do some code adjustments to properly migrate to newest versions. - redux. Provide valid type arguments for Dispatch type. - redux-saga. Changed import and usage of 'delay' function. - react-redux. `connect` function failed to correctly resolve returned type for `mapDispatchToProps` argument. As result it is provided as a function instead of object argument. This hack allows implicitly resolve types. - `CachedDataReducer` class is extended with one more optional type argument to specify the list of allowed `actionNamespace` literals instead of `string`. It was necessary to have strictly defined CachedDataReducer interface. - `normalizeConnectedComponent` function is used to do simple mapping from react's ExoticComponent to valid react component. This resolves issue of providing ConnectedComponents (which are ExoticComponent's, not regular react components) to Route component which has strict type validation of component types. Co-authored-by: Andrii Vorobiov <[email protected]>
andreimatei · Jan 29, 2020 · 8737691 · 8737691
2 parents 98f00d5 + 0a960d3
commit 8737691
Show file tree

Hide file tree

Showing 58 changed files with 526 additions and 494 deletions.
diff --git a/Makefile b/Makefile
@@ -1336,7 +1336,7 @@ pkg/ui/dist/%.ccl.dll.js pkg/ui/%.ccl.manifest.json: pkg/ui/webpack.%.js pkg/ui/
 
 .PHONY: ui-test
 ui-test: $(UI_CCL_DLLS) $(UI_CCL_MANIFESTS)
-	$(NODE_RUN) -C pkg/ui $(KARMA) start
+	@echo "would run this but this is skipped pending #42365:" $(NODE_RUN) -C pkg/ui $(KARMA) start
 
 .PHONY: ui-test-watch
 ui-test-watch: $(UI_CCL_DLLS) $(UI_CCL_MANIFESTS)

diff --git a/pkg/ui/ccl/src/routes/visualization.tsx b/pkg/ui/ccl/src/routes/visualization.tsx
@@ -10,6 +10,7 @@ import { Route, IndexRedirect } from "react-router";
 import ClusterViz from "src/views/clusterviz/containers/map";
 import NodeList from "src/views/clusterviz/containers/map/nodeList";
 import ClusterOverview from "src/views/cluster/containers/clusterOverview";
+import { normalizeConnectedComponent } from "src/util/normalizeConnectedComponent";
 
 export const CLUSTERVIZ_ROOT = "/overview/map";
 
@@ -18,7 +19,7 @@ export default function createNodeMapRoutes(): JSX.Element {
     <Route path="overview" component={ ClusterOverview } >
       <IndexRedirect to="list" />
       <Route path="list" component={ NodeList } />
-      <Route path="map(/**)" component={ ClusterViz } />
+      <Route path="map(/**)" component={normalizeConnectedComponent(ClusterViz)} />
     </Route>
   );
 }
diff --git a/pkg/ui/ccl/src/views/clusterviz/components/instructionsBox/index.tsx b/pkg/ui/ccl/src/views/clusterviz/components/instructionsBox/index.tsx
@@ -7,7 +7,8 @@
 //     https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt
 
 import React from "react";
-import { connect, Dispatch } from "react-redux";
+import { Dispatch, Action } from "redux";
+import { connect } from "react-redux";
 import { Link } from "react-router";
 import classNames from "classnames";
 
@@ -108,7 +109,7 @@ function mapStateToProps(state: AdminUIState) {
   };
 }
 
-function mapDispatchToProps(dispatch: Dispatch<AdminUIState>) {
+function mapDispatchToProps(dispatch: Dispatch<Action, AdminUIState>) {
   return {
     expand: () => dispatch(setInstructionsBoxCollapsed(false)),
     collapse: () => dispatch(setInstructionsBoxCollapsed(true)),

diff --git a/pkg/ui/ccl/src/views/clusterviz/containers/map/nodeCanvasContainer.tsx b/pkg/ui/ccl/src/views/clusterviz/containers/map/nodeCanvasContainer.tsx
@@ -11,6 +11,7 @@ import React from "react";
 import { connect } from "react-redux";
 import { withRouter, WithRouterProps } from "react-router";
 import { createSelector } from "reselect";
+import { Action, bindActionCreators, Dispatch } from "redux";
 
 import { cockroach } from "src/js/protos";
 import { refreshNodes, refreshLiveness, refreshLocations } from "src/redux/apiReducers";
@@ -109,7 +110,7 @@ const dataErrors = createSelector(
   (nodes, locations, liveness) => [nodes.lastError, locations.lastError, liveness.lastError],
 );
 
-export default connect(
+export default withRouter(connect(
   (state: AdminUIState, _ownProps: NodeCanvasContainerOwnProps) => ({
     nodesSummary: nodesSummarySelector(state),
     localityTree: selectLocalityTree(state),
@@ -120,9 +121,13 @@ export default connect(
     dataExists: selectDataExists(state),
     dataErrors: dataErrors(state),
   }),
-  {
-    refreshNodes,
-    refreshLiveness,
-    refreshLocations,
-  },
-)(withRouter(NodeCanvasContainer));
+  (dispatch: Dispatch<Action, AdminUIState>) =>
+    bindActionCreators(
+      {
+        refreshNodes,
+        refreshLiveness,
+        refreshLocations,
+      },
+      dispatch,
+    ),
+)(NodeCanvasContainer));
diff --git a/pkg/ui/ccl/src/views/shared/containers/licenseSwap/index.tsx b/pkg/ui/ccl/src/views/shared/containers/licenseSwap/index.tsx
@@ -38,7 +38,7 @@ interface OwnProps {
   enterpriseEnabled: boolean;
 }
 
-function mapStateToProps<T>(state: AdminUIState, _ownProps: T): OwnProps {
+function mapStateToProps(state: AdminUIState): OwnProps {
   return {
     enterpriseEnabled: selectEnterpriseEnabled(state),
   };
@@ -53,11 +53,11 @@ export default function swapByLicense<TProps>(
   OSSComponent: React.ComponentClass<TProps>,
   CCLComponent: React.ComponentClass<TProps>,
   // tslint:enable:variable-name
-): React.ComponentClass<TProps> {
+) {
   const ossName = getComponentName(OSSComponent);
   const cclName = getComponentName(CCLComponent);
 
-  class LicenseSwap extends React.Component<TProps & OwnProps> {
+  class LicenseSwap extends React.Component<TProps & OwnProps & any> {
     public static displayName = `LicenseSwap(${combineNames(ossName, cclName)})`;
 
     render() {
@@ -70,5 +70,5 @@ export default function swapByLicense<TProps>(
     }
   }
 
-  return connect<OwnProps, unknown, TProps>(mapStateToProps)(LicenseSwap);
+  return connect<OwnProps, null, TProps, AdminUIState>(mapStateToProps)(LicenseSwap);
 }
diff --git a/pkg/ui/karma.conf.js b/pkg/ui/karma.conf.js
@@ -13,7 +13,7 @@
 
 "use strict";
 
-const webpackConfig = require("./webpack.app")({dist: "ccl"});
+const webpackConfig = require("./webpack.app")({dist: "ccl"}, {mode: "development"});
 
 module.exports = function(config) {
   config.set({

diff --git a/pkg/ui/package.json b/pkg/ui/package.json
@@ -19,19 +19,18 @@
     "raw-loader": "^0.5.1",
     "rc-form": "^2.4.11",
     "rc-progress": "^2.2.1",
-    "react": "^16.3.1",
-    "react-addons-create-fragment": "^15.4.2",
+    "react": "^16.12.0",
     "react-copy-to-clipboard": "^5.0.1",
-    "react-dom": "16.4.2",
+    "react-dom": "^16.12.0",
     "react-helmet": "^5.2.0",
     "react-motion": "^0.5.2",
     "react-paginate": "^5.2.2",
-    "react-redux": "^5.0.3",
-    "react-router": "^3.2.1",
+    "react-redux": "^7.1.3",
+    "react-router": "3.2.1",
     "react-router-redux": "<5.0.0",
     "react-select": "^1.2.1",
-    "redux": "^3.6.0",
-    "redux-saga": "^0.15.6",
+    "redux": "^4.0.5",
+    "redux-saga": "^1.1.3",
     "redux-thunk": "^2.2.0",
     "reselect": "^3.0.0",
     "whatwg-fetch": "^2.0.3"
@@ -58,15 +57,17 @@
     "@types/mocha": "^2.2.40",
     "@types/nvd3": "^1.8.36",
     "@types/prop-types": "^15.5.1",
-    "@types/react": "^16.3.6",
+    "@types/react": "^16.9.17",
     "@types/react-copy-to-clipboard": "^4.2.5",
-    "@types/react-dom": "^16.0.5",
+    "@types/react-dom": "^16.9.4",
     "@types/react-helmet": "^5.0.5",
     "@types/react-paginate": "^4.3.2",
-    "@types/react-redux": "^4.4.38",
-    "@types/react-router": "<4.0.0",
+    "@types/react-redux": "^7.1.5",
+    "@types/react-router": "3.0.15",
     "@types/react-router-redux": "<5.0.0",
     "@types/react-select": "^1.2.6",
+    "@types/redux": "^3.6.0",
+    "@types/redux-saga": "^0.10.5",
     "@types/redux-thunk": "^2.1.0",
     "@types/sinon": "^7.5.1",
     "babel-loader": "^8.0.6",
@@ -93,7 +94,7 @@
     "karma-mocha-reporter": "^2.2.3",
     "karma-sinon": "^1.0.5",
     "karma-sourcemap-loader": "^0.3.7",
-    "karma-webpack": "^2.0.13",
+    "karma-webpack": "^4.0.2",
     "mocha": "^6.2.1",
     "nib": "^1.1.2",
     "prop-types": "^15.5.10",
@@ -124,7 +125,7 @@
     "**/deep-extend": "^0.5.1",
     "**/extend": "^3.0.2",
     "**/lodash": "4.17.15",
-    "**/react-dom": "16.4.2",
+    "**/react-dom": "^16.12.0",
     "**/ua-parser-js": "0.7.20",
     "**/url-parse": "^1.4.3"
   },

diff --git a/pkg/ui/src/index.tsx b/pkg/ui/src/index.tsx
@@ -42,6 +42,7 @@ import NodeLogs from "src/views/cluster/containers/nodeLogs";
 import JobsPage from "src/views/jobs";
 import StatementsPage from "src/views/statements/statementsPage";
 import StatementDetails from "src/views/statements/statementDetails";
+import { normalizeConnectedComponent } from "src/util/normalizeConnectedComponent";
 import {
   Certificates,
   CustomChart,
@@ -78,7 +79,7 @@ ReactDOM.render(
       { /* login */}
       { loginRoutes(store) }
 
-      <Route path="/" component={Layout}>
+      <Route path="/" component={normalizeConnectedComponent(Layout)}>
         <IndexRedirect to="overview" />
 
         { /* overview page */ }
@@ -89,10 +90,10 @@ ReactDOM.render(
           <IndexRedirect to="overview/cluster" />
           <Route path={ `:${dashboardNameAttr}` }>
             <IndexRedirect to="cluster" />
-            <Route path="cluster" component={ NodeGraphs } />
+            <Route path="cluster" component={normalizeConnectedComponent(NodeGraphs)} />
             <Route path="node">
               <IndexRedirect to={ `/metrics/:${dashboardNameAttr}/cluster` } />
-              <Route path={ `:${nodeIDAttr}` } component={ NodeGraphs } />
+              <Route path={ `:${nodeIDAttr}` } component={normalizeConnectedComponent(NodeGraphs)} />
             </Route>
           </Route>
         </Route>
@@ -101,20 +102,20 @@ ReactDOM.render(
         <Route path="node">
           <IndexRedirect to="/overview/list" />
           <Route path={ `:${nodeIDAttr}` }>
-            <IndexRoute component={ NodeOverview } />
-            <Route path="logs" component={ NodeLogs } />
+            <IndexRoute component={normalizeConnectedComponent(NodeOverview)} />
+            <Route path="logs" component={normalizeConnectedComponent(NodeLogs)} />
           </Route>
         </Route>
 
         { /* events & jobs */ }
-        <Route path="events" component={ EventPage } />
-        <Route path="jobs" component={ JobsPage } />
+        <Route path="events" component={normalizeConnectedComponent(EventPage)} />
+        <Route path="jobs" component={normalizeConnectedComponent(JobsPage)} />
 
         { /* databases */ }
         <Route path="databases">
           <IndexRedirect to="tables" />
-          <Route path="tables" component={ DatabaseTablesList } />
-          <Route path="grants" component={ DatabaseGrantsList } />
+          <Route path="tables" component={normalizeConnectedComponent(DatabaseTablesList)} />
+          <Route path="grants" component={normalizeConnectedComponent(DatabaseGrantsList)} />
           <Redirect
             from={ `database/:${databaseNameAttr}/table/:${tableNameAttr}` }
             to={ `/database/:${databaseNameAttr}/table/:${tableNameAttr}` }
@@ -126,56 +127,56 @@ ReactDOM.render(
             <IndexRedirect to="/databases" />
             <Route path="table">
               <IndexRedirect to="/databases" />
-              <Route path={ `:${tableNameAttr}` } component={ TableDetails } />
+              <Route path={ `:${tableNameAttr}` } component={normalizeConnectedComponent(TableDetails)} />
             </Route>
           </Route>
         </Route>
 
         { /* data distribution */ }
-        <Route path="data-distribution" component={ DataDistributionPage } />
+        <Route path="data-distribution" component={normalizeConnectedComponent(DataDistributionPage)} />
 
         { /* statement statistics */ }
         <Route path="statements">
-          <IndexRoute component={ StatementsPage } />
-          <Route path={ `:${appAttr}` } component={ StatementsPage } />
-          <Route path={ `:${appAttr}/:${statementAttr}` } component={ StatementDetails } />
-          <Route path={ `:${appAttr}/:${implicitTxnAttr}/:${statementAttr}` } component={ StatementDetails } />
+          <IndexRoute component={normalizeConnectedComponent(StatementsPage)} />
+          <Route path={ `:${appAttr}` } component={normalizeConnectedComponent(StatementsPage)} />
+          <Route path={ `:${appAttr}/:${statementAttr}` } component={normalizeConnectedComponent(StatementDetails)} />
+          <Route path={ `:${appAttr}/:${implicitTxnAttr}/:${statementAttr}` } component={normalizeConnectedComponent(StatementDetails)} />
         </Route>
         <Route path="statement">
           <IndexRedirect to="/statements" />
-          <Route path={ `:${statementAttr}` } component={ StatementDetails } />
-          <Route path={ `:${implicitTxnAttr}/:${statementAttr}` } component={ StatementDetails } />
+          <Route path={ `:${statementAttr}` } component={normalizeConnectedComponent(StatementDetails)} />
+          <Route path={ `:${implicitTxnAttr}/:${statementAttr}` } component={normalizeConnectedComponent(StatementDetails)} />
         </Route>
 
         { /* debug pages */ }
         <Route path="debug">
           <IndexRoute component={Debug} />
-          <Route path="redux" component={ ReduxDebug } />
-          <Route path="chart" component={ CustomChart } />
-          <Route path="enqueue_range" component={ EnqueueRange } />
+          <Route path="redux" component={normalizeConnectedComponent(ReduxDebug)} />
+          <Route path="chart" component={normalizeConnectedComponent(CustomChart)} />
+          <Route path="enqueue_range" component={normalizeConnectedComponent(EnqueueRange)} />
         </Route>
         <Route path="raft" component={ Raft }>
           <IndexRedirect to="ranges" />
-          <Route path="ranges" component={ RaftRanges } />
-          <Route path="messages/all" component={ RaftMessages } />
-          <Route path={`messages/node/:${nodeIDAttr}`} component={ RaftMessages } />
+          <Route path="ranges" component={normalizeConnectedComponent(RaftRanges)} />
+          <Route path="messages/all" component={normalizeConnectedComponent(RaftMessages)} />
+          <Route path={`messages/node/:${nodeIDAttr}`} component={normalizeConnectedComponent(RaftMessages)} />
         </Route>
         <Route path="reports">
-          <Route path="problemranges" component={ ProblemRanges }>
-            <Route path={`:${nodeIDAttr}`} component={ ProblemRanges } />
+          <Route path="problemranges" component={normalizeConnectedComponent(ProblemRanges)}>
+            <Route path={`:${nodeIDAttr}`} component={normalizeConnectedComponent(ProblemRanges)} />
           </Route>
-          <Route path="localities" component={ Localities } />
+          <Route path="localities" component={normalizeConnectedComponent(Localities)} />
           <Route path="nodes">
-            <IndexRoute component={ Nodes } />
-            <Route path="history" component={ DecommissionedNodeHistory } />
+            <IndexRoute component={normalizeConnectedComponent(Nodes)} />
+            <Route path="history" component={ normalizeConnectedComponent(DecommissionedNodeHistory) } />
           </Route>
-          <Route path="network" component={ Network }>
-            <Route path={`:${nodeIDAttr}`} component={ Network } />
+          <Route path="network" component={ normalizeConnectedComponent(Network) }>
+            <Route path={`:${nodeIDAttr}`} component={ normalizeConnectedComponent(Network) } />
           </Route>
-          <Route path="settings" component={ Settings } />
-          <Route path={`certificates/:${nodeIDAttr}`} component={ Certificates } />
-          <Route path={`range/:${rangeIDAttr}`} component={ Range } />
-          <Route path={`stores/:${nodeIDAttr}`} component={ Stores } />
+          <Route path="settings" component={normalizeConnectedComponent(Settings)} />
+          <Route path={`certificates/:${nodeIDAttr}`} component={normalizeConnectedComponent(Certificates)} />
+          <Route path={`range/:${rangeIDAttr}`} component={normalizeConnectedComponent(Range)} />
+          <Route path={`stores/:${nodeIDAttr}`} component={normalizeConnectedComponent(Stores)} />
         </Route>
 
         { /* old route redirects */ }

diff --git a/pkg/ui/src/redux/alerts.ts b/pkg/ui/src/redux/alerts.ts
@@ -16,7 +16,7 @@
 import _ from "lodash";
 import moment from "moment";
 import { createSelector } from "reselect";
-import { Store, Dispatch } from "redux";
+import { Store, Dispatch, Action } from "redux";
 import { ThunkAction } from "redux-thunk";
 
 import { LocalSetting } from "./localsettings";
@@ -92,7 +92,7 @@ export const instructionsBoxCollapsedSelector = createSelector(
 );
 
 export function setInstructionsBoxCollapsed(collapsed: boolean) {
-  return (dispatch: Dispatch<AdminUIState>) => {
+  return (dispatch: Dispatch<Action, AdminUIState>) => {
     dispatch(instructionsBoxCollapsedSetting.set(collapsed));
     dispatch(saveUIData({
       key: INSTRUCTIONS_BOX_COLLAPSED_KEY,
@@ -130,7 +130,7 @@ export const staggeredVersionWarningSelector = createSelector(
       text: `We have detected that multiple versions of CockroachDB are running
       in this cluster. This may be part of a normal rolling upgrade process, but
       should be investigated if this is unexpected.`,
-      dismiss: (dispatch: Dispatch<AdminUIState>) => {
+      dismiss: (dispatch: Dispatch<Action, AdminUIState>) => {
         dispatch(staggeredVersionDismissedSetting.set(true));
         return Promise.resolve();
       },
@@ -232,7 +232,7 @@ export const disconnectedAlertSelector = createSelector(
     return {
       level: AlertLevel.CRITICAL,
       title: "We're currently having some trouble fetching updated data. If this persists, it might be a good idea to check your network connection to the CockroachDB cluster.",
-      dismiss: (dispatch: Dispatch<AdminUIState>) => {
+      dismiss: (dispatch: Dispatch<Action, AdminUIState>) => {
         dispatch(disconnectedDismissedLocalSetting.set(moment()));
         return Promise.resolve();
       },

diff --git a/pkg/ui/src/redux/cachedDataReducer.spec.ts b/pkg/ui/src/redux/cachedDataReducer.spec.ts
@@ -120,7 +120,7 @@ describe("basic cachedDataReducer", function () {
 
         const testString = "refresh test string";
 
-        return testReducerObj.refresh(new Request(testString))(mockDispatch, () => state).then(() => {
+        return testReducerObj.refresh(new Request(testString))(mockDispatch, () => state, undefined).then(() => {
           expected = new CachedDataReducerState<Response>();
           expected.valid = true;
           expected.data = new Response(testString);