feat: Improve ref support on connectToStores (and remove refs from ot…

…her components) (#696)

package.json

@@ -16,7 +16,6 @@
     "lint": "eslint .",
     "test": "lerna run lint --since --stream && lerna run test --since --stream"
   },
-  "dependencies": {},
   "devDependencies": {
     "@babel/cli": "^7.11.6",
     "@babel/core": "^7.11.6",
@@ -43,6 +42,7 @@
     "pre-commit": "^1.0.7",
     "react": "^16.0.0",
     "react-dom": "^16.0.0",
+    "react-test-renderer": "^16.0.0",
     "shelljs": "^0.8.0",
     "sinon": "^9.0.1",
     "yargs": "^16.0.0"

packages/fluxible-addons-react/UPGRADE.md

@@ -51,13 +51,18 @@ If you were using `provideContext` to provide other context data not
 related to fluxible itself, you will need to provide your own
 solution to achieve the same result as before.
 
+#### Ref support removed
+
+This hoc does not include `wrappedComponent` ref anymore.
+
 ### connectToStores
 
-`connectToStores(Component, stores, getStateFromStores, customContextTypes)` -> `connectToStores(Component, stores, getStateFromStores)`
+`connectToStores(Component, stores, getStateFromStores, customContextTypes)` -> `connectToStores(Component, stores, getStateFromStores, options)`
 
 Since the new React API doesn't rely on PropTypes anymore, there is no
-need to specify customContextTypes to extract plugins context from the
-fluxible context since all plugins will be available.
+need to specify the `customContextTypes` param to extract plugins
+context from the fluxible context. All plugins component context are
+available.
 
 **Before:**
 
@@ -67,30 +72,80 @@ const customContextTypes = {
     pluginFoo: PropTypes.object
 };
 
-connectToStores(Component, customContextTypes)
+connectToStores(Component, stores, getStateFromStores, customContextTypes)
 ```
 
 **After:**
 
 ```javascript
-connectToStores(Component)
+connectToStores(Component, stores, getStateFromStores)
 ```
 
-If you were relying in other contextTypes that were not included in
-`provideContext` (non fluxible plugins), you will need to find another
-way to have it available in `getStateFromStores`. Since the second
-argument of `getStateFromStores` is the props passed to the wrapped
-component, you can create your own high order component that passes
-the required context as props to your connected component:
+#### Ref support improved
+
+`connectToStores` now returns a `React.forwardRef` component instead
+of internally attaching the `wrappedComponent` ref to the wrapped
+component. This means that you can now pass a ref to the connected
+component that it will be forwarded to the wrapped component.
+
+However, in order to have your prop forwarded, you need to explicitly
+tell it to `connectToStores` by setting `forwardRef` to `true` in the
+`options` param.
+
 
+**Before**
 ```javascript
-const customContextTypes = {
-    foo: PropTypes.string,
-    bar: PropTypes.object
-};
+// Only class components would get access to a ref
+class CustomInput extends React.Component {
+    constructor() {
+        this.ref = React.createRef();
+    }
+
+    render() {
+        return <input ref={this.ref} {...this.props} />
+    }
+}
 
-export default injectContext(customContextTypes, connectToStores(Component));
+const ConnectedInput = connectToStores(CustomInput, stores, getStateFromStores);
+
+class App extends React.Component {
+    constructor() {
+        this.ref = React.createRef();
+    }
+
+    componentDidMount() {
+        this.ref.current.wrappedComponent.ref.current.focus()
+    }
+
+    render() {
+        return <ConnectedInput ref={this.ref} />
+    }
+}
 ```
 
-You can read more about how to create your own high order components
-at React [official documentation](https://reactjs.org/docs/higher-order-components.html).
+**After**
+```javascript
+// Besides classes, it's now possible to forward refs to functional components.
+const CustomInput = React.forwardRef((props, ref) => <input ref={ref} {...props} />);
+
+const ConnectedInput = connectToStores(
+    CustomInput,
+    stores,
+    getStateFromStores,
+    { forwardRef: true }
+);
+
+class App extends React.Component {
+    constructor() {
+        this.ref = React.createRef();
+    }
+
+    componentDidMount() {
+        this.ref.current.focus()
+    }
+
+    render() {
+        return <ConnectedInput ref={this.ref} />
+    }
+}
+```

packages/fluxible-addons-react/src/connectToStores.js

@@ -2,40 +2,43 @@
  * Copyright 2015, Yahoo Inc.
  * Copyrights licensed under the New BSD License. See the accompanying LICENSE file for terms.
  */
-import { Component as ReactComponent, createRef, createElement } from 'react';
+import { Component as ReactComponent, createElement, forwardRef } from 'react';
 import hoistNonReactStatics from 'hoist-non-react-statics';
 import { FluxibleContext } from './FluxibleContext';
 
 /**
- * Registers change listeners and retrieves state from stores using the `getStateFromStores`
- * method. Concept provided by Dan Abramov via
- * https://medium.com/@dan_abramov/mixins-are-dead-long-live-higher-order-components-94a0d2f9e750
+ * @callback getStateFromStores
+ * @param {FluxibleContext} context - Fluxible component context.
+ * @param {Object} ownProps - The props that the component received.
+ * @returns {Object} props - The props that should be passed to the component.
+ */
+
+/**
+ * HOC that registers change listeners and retrieves state from stores
+ * using the `getStateFromStores` method.
  *
  * Example:
- *   connectToStores(Component, [FooStore], {
- *       FooStore: function (store, props) {
- *           return {
- *               foo: store.getFoo()
- *           }
- *       }
+ *   connectToStores(Component, [FooStore], (context, props) => ({
+ *       foo: context.getStore(FooStore).getFoo(),
+ *       onClick: () => context.executeAction(fooAction, props)
  *   })
  *
- * @method connectToStores
- * @param {React.Component} [Component] component to pass state as props to.
- * @param {array} stores List of stores to listen for changes
- * @param {function} getStateFromStores function that receives all stores and should return
- *      the full state object. Receives `stores` hash and component `props` as arguments
- * @returns {React.Component} or {Function} if using decorator pattern
+ * @function connectToStores
+ * @param {React.Component} Component - The component to pass state as props to.
+ * @param {array} stores - List of stores to listen for changes.
+ * @param {getStateFromStores} getStateFromStores - The main function that must map the context into props.
+ * @param {Object} [options] - options to tweak the HOC.
+ * @param {boolean} options.forwardRef - If true, forwards a ref to the wrapped component.
+ * @returns {React.Component} ConnectedComponent - A component connected to the stores.
  */
-function connectToStores(Component, stores, getStateFromStores) {
+function connectToStores(Component, stores, getStateFromStores, options) {
     class StoreConnector extends ReactComponent {
         constructor(props, context) {
             super(props, context);
             this._isMounted = false;
             this._onStoreChange = this._onStoreChange.bind(this);
             this.getStateFromStores = this.getStateFromStores.bind(this);
             this.state = this.getStateFromStores();
-            this.wrappedElementRef = createRef();
         }
 
         getStateFromStores(props) {
@@ -51,35 +54,47 @@ function connectToStores(Component, stores, getStateFromStores) {
 
         componentDidMount() {
             this._isMounted = true;
-            stores.forEach(Store => this.context.getStore(Store).on('change', this._onStoreChange));
+            stores.forEach((Store) =>
+                this.context.getStore(Store).on('change', this._onStoreChange)
+            );
         }
 
         componentWillUnmount() {
             this._isMounted = false;
-            stores.forEach(Store => this.context.getStore(Store).removeListener('change', this._onStoreChange));
+            stores.forEach((Store) =>
+                this.context
+                    .getStore(Store)
+                    .removeListener('change', this._onStoreChange)
+            );
         }
 
         UNSAFE_componentWillReceiveProps(nextProps) {
             this.setState(this.getStateFromStores(nextProps));
         }
 
         render() {
-            const props = (Component.prototype && Component.prototype.isReactComponent)
-                ? {ref: this.wrappedElementRef}
-                : null;
-            return createElement(Component, {...this.props, ...this.state, ...props});
+            return createElement(Component, {
+                ref: this.props.fluxibleRef,
+                ...this.props,
+                ...this.state,
+            });
         }
     }
 
-    StoreConnector.displayName = `storeConnector(${Component.displayName || Component.name || 'Component'})`;
-
     StoreConnector.contextType = FluxibleContext;
 
-    StoreConnector.WrappedComponent = Component;
-
-    hoistNonReactStatics(StoreConnector, Component);
+    const forwarded = forwardRef((props, ref) =>
+        createElement(StoreConnector, {
+            ...props,
+            fluxibleRef: options?.forwardRef ? ref : null,
+        })
+    );
+    forwarded.displayName = `storeConnector(${
+        Component.displayName || Component.name || 'Component'
+    })`;
+    forwarded.WrappedComponent = Component;
 
-    return StoreConnector;
+    return hoistNonReactStatics(forwarded, Component);
 }
 
 export default connectToStores;

packages/fluxible-addons-react/src/provideContext.js

@@ -2,7 +2,7 @@
  * Copyright 2015, Yahoo Inc.
  * Copyrights licensed under the New BSD License. See the accompanying LICENSE file for terms.
  */
-import { Component as ReactComponent, createRef, createElement } from 'react';
+import { Component as ReactComponent, createElement } from 'react';
 import { object } from 'prop-types';
 import hoistNonReactStatics from 'hoist-non-react-statics';
 import { FluxibleProvider } from './FluxibleContext';
@@ -23,20 +23,11 @@ function provideContext(Component) {
     class ContextProvider extends ReactComponent {
         constructor(props) {
             super(props);
-            this.wrappedElementRef = createRef();
         }
 
         render() {
-            const props =
-                Component.prototype && Component.prototype.isReactComponent
-                    ? { ref: this.wrappedElementRef }
-                    : null;
-
             const { context } = this.props;
-            const children = createElement(Component, {
-                ...this.props,
-                ...props,
-            });
+            const children = createElement(Component, this.props);
             return createElement(FluxibleProvider, { context }, children);
         }
     }