[wip] Use the built-in connect function rather than explicitConnect

package.json

@@ -36,14 +36,16 @@
     "test-coverage": "run-s test-build-coverage test-serve-coverage",
     "test-alex": "alex ./docs-* *.md",
     "test-lockfile": "lockfile-lint --path yarn.lock --allowed-hosts yarn --validate-https",
-    "test-debug": "node --inspect-brk node_modules/.bin/jest --runInBand"
+    "test-debug": "node --inspect-brk node_modules/.bin/jest --runInBand",
+    "ts": "tsc"
   },
   "license": "MPL-2.0",
   "repository": {
     "type": "git",
     "url": "https://github.com/firefox-devtools/profiler"
   },
   "dependencies": {
+    "@types/react-redux": "^7.1.11",
     "array-move": "^3.0.1",
     "array-range": "^1.0.1",
     "clamp": "^1.0.1",
@@ -71,6 +73,7 @@
     "redux-logger": "^3.0.6",
     "redux-thunk": "^2.2.0",
     "reselect": "^4.0.0",
+    "typescript": "^4.0.5",
     "url": "^0.11.0",
     "weaktuplemap": "^1.0.0"
   },

src/components/app/DragAndDrop.js

@@ -7,8 +7,7 @@
 import * as React from 'react';
 import classNames from 'classnames';
 import { retrieveProfileFromFile } from 'firefox-profiler/actions/receive-profile';
-import type { ConnectedProps } from 'firefox-profiler/utils/connect';
-import explicitConnect from 'firefox-profiler/utils/connect';
+import { connect } from 'react-redux';
 
 import {
   startDragging,
@@ -21,6 +20,7 @@ import {
   getIsDragAndDropOverlayRegistered,
   getIsNewProfileLoadAllowed,
 } from 'firefox-profiler/selectors/app';
+import type { ConnectedProps, State } from 'firefox-profiler/types';
 
 import './DragAndDrop.css';
 
@@ -44,6 +44,17 @@ type DispatchProps = {|
   +stopDragging: typeof stopDragging,
 |};
 
+const mapStateToProps = (state: State): StateProps => ({
+  isNewProfileLoadAllowed: getIsNewProfileLoadAllowed(state),
+  useDefaultOverlay: !getIsDragAndDropOverlayRegistered(state),
+});
+
+const mapDispatchToProps: DispatchProps = {
+  retrieveProfileFromFile,
+  startDragging,
+  stopDragging,
+};
+
 type Props = ConnectedProps<OwnProps, StateProps, DispatchProps>;
 
 /**
@@ -139,20 +150,10 @@ class DragAndDropImpl extends React.PureComponent<Props> {
   }
 }
 
-export const DragAndDrop = explicitConnect<OwnProps, StateProps, DispatchProps>(
-  {
-    mapStateToProps: state => ({
-      isNewProfileLoadAllowed: getIsNewProfileLoadAllowed(state),
-      useDefaultOverlay: !getIsDragAndDropOverlayRegistered(state),
-    }),
-    mapDispatchToProps: {
-      retrieveProfileFromFile,
-      startDragging,
-      stopDragging,
-    },
-    component: DragAndDropImpl,
-  }
-);
+export const DragAndDrop = connect<OwnProps, StateProps, DispatchProps>(
+  mapStateToProps,
+  mapDispatchToProps
+)(DragAndDropImpl);
 
 type OverlayOwnProps = {|
   +isDefault?: boolean,
@@ -207,18 +208,21 @@ class DragAndDropOverlayImpl extends React.PureComponent<OverlayProps> {
   }
 }
 
-export const DragAndDropOverlay = explicitConnect<
+const overlayMapStateToProps = (state: State): OverlayStateProps => ({
+  isDragging: getIsDragAndDropDragging(state),
+  isNewProfileLoadAllowed: getIsNewProfileLoadAllowed(state),
+});
+
+const overlayMapDispatchToProps: OverlayDispatchProps = {
+  registerDragAndDropOverlay,
+  unregisterDragAndDropOverlay,
+};
+
+export const DragAndDropOverlay = connect<
   OverlayOwnProps,
   OverlayStateProps,
   OverlayDispatchProps
->({
-  mapStateToProps: state => ({
-    isDragging: getIsDragAndDropDragging(state),
-    isNewProfileLoadAllowed: getIsNewProfileLoadAllowed(state),
-  }),
-  mapDispatchToProps: {
-    registerDragAndDropOverlay,
-    unregisterDragAndDropOverlay,
-  },
-  component: DragAndDropOverlayImpl,
-});
+>(
+  overlayMapStateToProps,
+  overlayMapDispatchToProps
+)(DragAndDropOverlayImpl);

src/test/types/react-redux-connect.js

@@ -0,0 +1,240 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
+// @flow
+
+import * as React from 'react';
+import { connect } from 'react-redux';
+
+import type {
+  State,
+  Action,
+  ThunkAction,
+  Dispatch,
+  GetState,
+  ConnectedProps,
+} from 'firefox-profiler/types';
+/* eslint-disable no-unused-vars */
+import {
+  coerce,
+  coerceThunk0,
+  coerceThunk1,
+  coerceThunk2,
+} from '../../utils/flow';
+
+// Use this any value to create fake variables as a type. Consider using
+// `declare var myVariables: MyType;` instead. However, it can sometimes be clearer to
+// create values inline, or from pre-existing type definitions. In addition,
+// `declare var` is not correctly lexically scoped.
+const ANY_VALUE = coerce<number, any>(0);
+const numberValue: number = 5;
+const stringValue: string = 'string';
+
+/**
+ * These type tests create various values that should all type check correctly to show
+ * that the react-redux system is working correctly.
+ */
+
+type OwnProps = {|
+  +ownPropString: string,
+  +ownPropNumber: number,
+|};
+
+type StateProps = {|
+  +statePropString: string,
+  +statePropNumber: number,
+|};
+
+type ExampleActionCreator = string => Action;
+type ExampleThunkActionCreator = string => ThunkAction<number>;
+
+type DispatchProps = {|
+  +dispatchString: ExampleActionCreator,
+  // TODO - Switch to TypeScript and use a coerceThunk strategy, and infer the
+  // DispatchProps.
+  +dispatchThunk: ExampleThunkActionCreator,
+|};
+
+type Props = ConnectedProps<OwnProps, StateProps, DispatchProps>;
+
+class ExampleComponent extends React.PureComponent<Props> {
+  render() {
+    // Ensure that the React component has the correct types inside of it.
+    (this.props.ownPropString: string);
+    (this.props.ownPropNumber: number);
+    (this.props.statePropString: string);
+    (this.props.statePropNumber: number);
+
+    // The action creators are properly wrapped by dispatch.
+    (this.props.dispatchString: string => Action);
+
+    // DispatchThunk is currently broken, but can be properly fixed by inferring
+    // the type using a DeThunk strategy if and when we switch to TypeScript.
+    //
+    // ```js
+    //   const mapStateToProps = (state: State) => ({
+    //     foo: getFoo(state),
+    //     bar: getBar(state),
+    //   });
+
+    //   const mapDispatchToProps = {
+    //     // Fakes the transform:
+    //     //   () => (D, GS) => Return
+    //     //   () => Return
+    //     doThunkAction0: coerceThunk0(doThunkAction0),
+    //     // Fakes the transform:
+    //     //   (A) => (D, GS) => Return
+    //     //   (A) => Return
+    //     doThunkAction1: coerceThunk1(doThunkAction1),
+    //     // Fakes the transform:
+    //     //   (A, B) => (D, GS) => Return
+    //     //   (A, B) => Return
+    //     doThunkAction2: coerceThunk2(doThunkAction2),
+    //   };
+
+    //   type StateProps = ReturnType<typeof mapStateToProps>;
+    //   type DispatchProps = typeof mapDispatchToProps;
+    //   type Props = ConnectedProps<{}, StateProps, DispatchProps>;
+    // ```
+
+    // $FlowFixMe - See comment above
+    (this.props.dispatchThunk: string => number);
+    // $FlowFixMe - See comment above
+    (this.props.dispatchThunk('foo'): number);
+
+    return null;
+  }
+}
+
+const validMapStateToProps = (state: State, ownProps: OwnProps): StateProps => {
+  return {
+    statePropString: 'string',
+    statePropNumber: 0,
+  };
+};
+
+type DispatchToProps = {|
+  +dispatchString: string => Action,
+  +dispatchThunk: string => ThunkAction<number>,
+|};
+
+declare var validDispatchToProps: DispatchToProps;
+
+// This value also serves as a test for the common case of creating a component
+// with valid values.
+const ConnectedExampleComponent = connect<OwnProps, StateProps, DispatchProps>(
+  validMapStateToProps,
+  validDispatchToProps
+)(ExampleComponent);
+
+type DeThunk0Expected = () => number;
+type DeThunk1Expected = (a: string) => number;
+type DeThunk2Expected = (a: string, b: number) => number;
+{
+  // Test the de-thunk process.
+  const thunk0 = () => (dispatch: Dispatch, getState: GetState) => numberValue;
+  const thunk1 = (a: string) => (dispatch: Dispatch, getState: GetState) =>
+    numberValue;
+  const thunk2 = (a: string, b: number) => (
+    dispatch: Dispatch,
+    getState: GetState
+  ) => numberValue;
+
+  const deThunk0Actual: DeThunk0Expected = coerceThunk0(thunk0);
+  const deThunk1Actual: DeThunk1Expected = coerceThunk1(thunk1);
+  const deThunk2Actual: DeThunk2Expected = coerceThunk2(thunk2);
+
+  // $FlowExpectError
+  const deThunk0Error: () => void = coerceThunk0(thunk0);
+  // $FlowExpectError
+  const deThunk1Error: () => void = coerceThunk1(thunk0);
+  // $FlowExpectError
+  const deThunk2Error: () => void = coerceThunk0(thunk0);
+}
+
+{
+  // Test that mapStateToProps will error out if provided an extra value.
+  connect<OwnProps, StateProps, DispatchProps>(
+    // $FlowExpectError
+    (state: State): StateProps => ({
+      statePropString: 'string',
+      statePropNumber: 0,
+      extraValue: null,
+    }),
+    validDispatchToProps
+  )(ExampleComponent);
+}
+
+{
+  // Test that mapStateToProps will error if provided an extra value.
+  connect<OwnProps, StateProps, DispatchProps>(
+    (state: State): StateProps => ({
+      statePropString: 'string',
+      // $FlowExpectError
+      statePropNumber: 'not a number',
+    }),
+    validDispatchToProps
+  )(ExampleComponent);
+}
+
+{
+  // Test that mapDispatchToProps will error if a value is omitted.
+  connect<OwnProps, StateProps, DispatchProps>(
+    validMapStateToProps,
+    // $FlowExpectError
+    (ANY_VALUE: {|
+      +dispatchThunk: string => ThunkAction<number>,
+    |})
+  )(ExampleComponent);
+}
+
+{
+  // Test that mapDispatchToProps will error if a variable type definition is wrong.
+  connect<OwnProps, StateProps, DispatchProps>(
+    validMapStateToProps,
+    (ANY_VALUE: {|
+      // $FlowExpectError
+      +dispatchString: string => string,
+      +dispatchThunk: string => ThunkAction<number>,
+    |})
+  )(ExampleComponent);
+}
+
+{
+  // Test that mapDispatchToProps will error if an extra property is given.
+  connect<OwnProps, StateProps, DispatchProps>(
+    validMapStateToProps,
+    // $FlowExpectError
+    (ANY_VALUE: {|
+      ...typeof validDispatchToProps,
+      +extraProperty: string => string,
+    |})
+  )(ExampleComponent);
+}
+
+{
+  // The connected component correctly takes OwnProps.
+  <ConnectedExampleComponent ownPropString="string" ownPropNumber={0} />;
+}
+
+{
+  // The connected component must not accept more props.
+  // $FlowExpectError
+  <ConnectedExampleComponent
+    ownPropString="string"
+    ownPropNumber={0}
+    ownPropsExtra={0}
+  />;
+}
+
+{
+  // It throws an error when an OwnProps is incorrect.
+  // $FlowExpectError
+  <ConnectedExampleComponent ownPropString={0} ownPropNumber={0} />;
+}
+
+{
+  // It throws an error if no OwnProps are provided.
+  // $FlowExpectError
+  <ConnectedExampleComponent />;
+}