From 8c89862a2ee8307863ba91f20db9a86e2a9c9496 Mon Sep 17 00:00:00 2001
From: vvo <vincent.voyer@algolia.com>
Date: Wed, 17 Feb 2016 13:49:08 +0100
Subject: [PATCH] perf(autoHideContainer): stop re-creating React components

The previous strategy for autoHideContainer was to either return an
empty div or the wrapped element.

So when the widget cycle was like this: init => hide (no results) =>
show (results) then we would re-create instances of React components because of a
different render() output in autoHideContainer.

This I discovered it while going to do #835: when the widgets would
hide => show we would lose the wrapped React component state resulting
in lost collaspable state.

Not only this commit will permit doing #835 easily but it also somehow
enhance performance since I also added a shouldComponentUpdate
strategy.

Added more precise tests on this decorator/HOC.
---
 .../__tests__/autoHideContainer-test.js       | 78 +++++++++++++++----
 src/decorators/autoHideContainer.js           | 12 ++-
 2 files changed, 69 insertions(+), 21 deletions(-)

diff --git a/src/decorators/__tests__/autoHideContainer-test.js b/src/decorators/__tests__/autoHideContainer-test.js
index 9b4e720358..25769e7b91 100644
--- a/src/decorators/__tests__/autoHideContainer-test.js
+++ b/src/decorators/__tests__/autoHideContainer-test.js
@@ -1,36 +1,80 @@
 /* eslint-env mocha */
 
 import React from 'react';
+import ReactDOM from 'react-dom';
 import expect from 'expect';
 import TestUtils from 'react-addons-test-utils';
 import TestComponent from './TestComponent';
 import autoHideContainer from '../autoHideContainer';
+import jsdom from 'jsdom-global';
+import sinon from 'sinon';
 
 import expectJSX from 'expect-jsx';
 expect.extend(expectJSX);
 
 describe('autoHideContainer', () => {
-  let renderer;
+  beforeEach(function() {this.jsdom = jsdom();});
+  afterEach(function() {this.jsdom();});
 
-  beforeEach(() => {
-    let {createRenderer} = TestUtils;
-    renderer = createRenderer();
-  });
+  let props = {};
 
   it('should render autoHideContainer(<TestComponent />)', () => {
-    let out = render();
-    expect(out).toEqualJSX(<TestComponent />);
+    let {createRenderer} = TestUtils;
+    let renderer = createRenderer();
+    props.hello = 'son';
+    let AutoHide = autoHideContainer(TestComponent);
+    renderer.render(<AutoHide {...props} />);
+    let out = renderer.getRenderOutput();
+    expect(out).toEqualJSX(<TestComponent hello="son" />);
   });
 
-  it('should not render autoHideContainer(<TestComponent />)', () => {
-    let out = render({shouldAutoHideContainer: true});
-    expect(out).toEqualJSX(<div />);
-  });
+  context('props.shouldAutoHideContainer', () => {
+    let AutoHide;
+    let component;
+    let container;
 
-  function render(props = {}) {
-    let AutoHide = autoHideContainer(TestComponent);
-    renderer.render(<AutoHide {...props} />);
-    return renderer.getRenderOutput();
-  }
-});
+    beforeEach(() => {
+      AutoHide = autoHideContainer(TestComponent);
+      container = document.createElement('div');
+      props = {hello: 'mom'};
+      component = ReactDOM.render(<AutoHide {...props} />, container);
+    });
 
+    it('creates a component', () => expect(component).toExist());
+
+    it('shows the container at first', () => {
+      expect(container.style.display).toNotEqual('none');
+    });
+
+    context('when set to true', () => {
+      beforeEach(() => {
+        sinon.spy(component, 'render');
+        props.shouldAutoHideContainer = true;
+        ReactDOM.render(<AutoHide {...props} />, container);
+      });
+
+      it('hides the container', () => {
+        expect(container.style.display).toEqual('none');
+      });
+
+      it('does not call component.render()', () => {
+        expect(component.render.called).toBe(false);
+      });
+
+      context('when set back to false', () => {
+        beforeEach(() => {
+          props.shouldAutoHideContainer = false;
+          ReactDOM.render(<AutoHide {...props} />, container);
+        });
+
+        it('shows the container', () => {
+          expect(container.style.display).toNotEqual('none');
+        });
+
+        it('calls component.render()', () => {
+          expect(component.render.calledOnce).toBe(true);
+        });
+      });
+    });
+  });
+});
diff --git a/src/decorators/autoHideContainer.js b/src/decorators/autoHideContainer.js
index a1dff0e0b0..f8b0c5f981 100644
--- a/src/decorators/autoHideContainer.js
+++ b/src/decorators/autoHideContainer.js
@@ -11,19 +11,23 @@ function autoHideContainer(ComposedComponent) {
     }
 
     componentWillReceiveProps(nextProps) {
+      if (this.props.shouldAutoHideContainer === nextProps.shouldAutoHideContainer) {
+        return;
+      }
+
       this._hideOrShowContainer(nextProps);
     }
 
+    shouldComponentUpdate(nextProps) {
+      return nextProps.shouldAutoHideContainer === false;
+    }
+
     _hideOrShowContainer(props) {
       let container = ReactDOM.findDOMNode(this).parentNode;
       container.style.display = (props.shouldAutoHideContainer === true) ? 'none' : '';
     }
 
     render() {
-      if (this.props.shouldAutoHideContainer === true) {
-        return <div />;
-      }
-
       return <ComposedComponent {...this.props} />;
     }
   }