fix(addons-react): prevent fluxibleRef to leak in connected components (

#752)
yahoo · Dec 28, 2021 · afd76ef · afd76ef
1 parent 2ba3719
commit afd76ef
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 16 deletions.
diff --git a/packages/fluxible-addons-react/src/connectToStores.js b/packages/fluxible-addons-react/src/connectToStores.js
@@ -62,11 +62,12 @@ function connectToStores(Component, stores, getStateFromStores, options) {
         }
 
         render() {
-            const storeState = getStateFromStores(this.context, this.props);
+            const { fluxibleRef, ...props } = this.props;
+            const storeState = getStateFromStores(this.context, props);
 
             return createElement(Component, {
-                ref: this.props.fluxibleRef,
-                ...this.props,
+                ref: fluxibleRef,
+                ...props,
                 ...storeState,
             });
         }
@@ -75,10 +76,12 @@ function connectToStores(Component, stores, getStateFromStores, options) {
     StoreConnector.contextType = FluxibleComponentContext;
 
     const forwarded = forwardRef((props, ref) =>
-        createElement(StoreConnector, {
-            ...props,
-            fluxibleRef: options?.forwardRef ? ref : null,
-        })
+        createElement(
+            StoreConnector,
+            options?.forwardRef
+                ? Object.assign({ fluxibleRef: ref }, props)
+                : props
+        )
     );
     forwarded.displayName = `storeConnector(${
         Component.displayName || Component.name || 'Component'

diff --git a/packages/fluxible-addons-react/tests/unit/lib/FluxibleComponentContext.test.js b/packages/fluxible-addons-react/tests/unit/lib/FluxibleComponentContext.test.js
@@ -1,7 +1,7 @@
 const React = require('react');
 const { useContext } = require('react');
 const { renderToString } = require('react-dom/server');
-const { FluxibleComponent, FluxibleComponentContext } = require('../../../');
+const { FluxibleProvider, FluxibleComponentContext } = require('../../../');
 
 describe('FluxibleComponentContext', () => {
     it('provides access to getStore and executeAction', () => {
@@ -20,9 +20,9 @@ describe('FluxibleComponentContext', () => {
         };
 
         renderToString(
-            <FluxibleComponent context={context}>
+            <FluxibleProvider context={context}>
                 <Component />
-            </FluxibleComponent>
+            </FluxibleProvider>
         );
 
         expect(context.getStore).toHaveBeenCalledTimes(1);

diff --git a/packages/fluxible-addons-react/tests/unit/lib/connectToStores.test.js b/packages/fluxible-addons-react/tests/unit/lib/connectToStores.test.js
@@ -8,7 +8,7 @@ const {
 } = require('react');
 const TestRenderer = require('react-test-renderer');
 const createMockComponentContext = require('fluxible/utils/createMockComponentContext');
-const { connectToStores, FluxibleComponent } = require('../../../');
+const { connectToStores, FluxibleProvider } = require('../../../');
 const FooStore = require('../../fixtures/stores/FooStore');
 const BarStore = require('../../fixtures/stores/BarStore');
 
@@ -40,15 +40,18 @@ const renderComponent = (Component, ref) => {
     const context = createMockComponentContext({ stores });
 
     const app = TestRenderer.create(
-        <FluxibleComponent context={context}>
+        <FluxibleProvider context={context}>
             <Component ref={ref} />
-        </FluxibleComponent>
+        </FluxibleProvider>
     );
 
     return { app, context };
 };
 
-const getComponent = (app) => app.root.findByType(DumbComponent);
+const getComponent = (app, component) => app.root.findByType(component);
+
+const getStoreConnector = (app) =>
+    app.root.find((node) => node.type.name === 'StoreConnector');
 
 describe('fluxible-addons-react', () => {
     describe('connectToStores', () => {
@@ -80,7 +83,7 @@ describe('fluxible-addons-react', () => {
 
         it('should forward props from getStateFromStores to component', () => {
             const { app } = renderComponent(ConnectedComponent);
-            const component = getComponent(app);
+            const component = getComponent(app, DumbComponent);
 
             expect(component.props.foo).toBe('bar');
             expect(component.props.bar).toBe('baz');
@@ -89,7 +92,7 @@ describe('fluxible-addons-react', () => {
 
         it('should listen to store changes', () => {
             const { app } = renderComponent(ConnectedComponent);
-            const component = getComponent(app);
+            const component = getComponent(app, DumbComponent);
 
             component.props.onClick();
 
@@ -156,6 +159,30 @@ describe('fluxible-addons-react', () => {
                 renderComponent(ConnectedForwardComponent, ref2);
                 expect(ref2.current.number).toBe(24);
             });
+
+            it('does not pass fluxibleRef to StoreConnector if ref is disabled', () => {
+                const ref = createRef(null);
+                const { app } = renderComponent(ConnectedComponent, ref);
+
+                const connector = app.root.find(
+                    (node) => node.type.name === 'StoreConnector'
+                );
+                expect(connector.props.fluxibleRef).toBe(undefined);
+
+                const component = getComponent(app, ConnectedComponent);
+                expect(component.props.fluxibleRef).toBe(undefined);
+            });
+
+            it('does not leak fluxibleRef to inner component', () => {
+                const ref = createRef(null);
+                const { app } = renderComponent(ConnectedClassComponent, ref);
+
+                const connector = getStoreConnector(app);
+                expect(connector.props.fluxibleRef).not.toBe(undefined);
+
+                const component = getComponent(app, ConnectedClassComponent);
+                expect(component.props.fluxibleRef).toBe(undefined);
+            });
         });
     });
 });