fix(InstantSearch): dont fire request/onsearchStateChange when unmoun…

…ting (#26)

packages/react-instantsearch/src/core/InstantSearch.js

@@ -50,8 +50,8 @@ class InstantSearch extends Component {
     super(props);
 
     this.isControlled = Boolean(props.searchState);
-
     const initialState = this.isControlled ? props.searchState : {};
+    this.isUnmounting = false;
 
     this.aisManager = createInstantSearchManager({
       indexName: props.indexName,
@@ -73,6 +73,11 @@ class InstantSearch extends Component {
     }
   }
 
+  componentWillUnmount() {
+    this.isUnmounting = true;
+    this.aisManager.skipSearch();
+  }
+
   getChildContext() {
     // If not already cached, cache the bound methods so that we can forward them as part
     // of the context.
@@ -115,7 +120,7 @@ class InstantSearch extends Component {
   }
 
   onSearchStateChange(searchState) {
-    if (this.props.onSearchStateChange) {
+    if (this.props.onSearchStateChange && !this.isUnmounting) {
       this.props.onSearchStateChange(searchState);
     }
   }

packages/react-instantsearch/src/core/InstantSearch.test.js

@@ -206,6 +206,31 @@ describe('InstantSearch', () => {
     expect(context.ais.widgetsManager).toBe(ism.widgetsManager);
   });
 
+  it('onSearchStateChange should not be called and search should be skipped if the widget is unmounting', () => {
+    const ism = {
+      skipSearch: jest.fn(),
+    };
+    createInstantSearchManager.mockImplementation(() => ism);
+    const onSearchStateChangeMock = jest.fn();
+    const wrapper = mount(
+      <InstantSearch
+        {...DEFAULT_PROPS}
+        onSearchStateChange={onSearchStateChangeMock}
+      >
+        <div />
+      </InstantSearch>
+    );
+    const {
+      ais: { onSearchStateChange },
+    } = wrapper.instance().getChildContext();
+
+    wrapper.unmount();
+    onSearchStateChange({});
+
+    expect(onSearchStateChangeMock.mock.calls.length).toBe(0);
+    expect(ism.skipSearch.mock.calls.length).toBe(1);
+  });
+
   describe('createHrefForState', () => {
     it('passes through to createURL when it is defined', () => {
       const widgetsIds = [];

packages/react-instantsearch/src/core/createConnector.js

@@ -111,6 +111,7 @@ export default function createConnector(connectorDesc) {
           if (isWidget) {
             // Since props might have changed, we need to re-run getSearchParameters
             // and getMetadata with the new props.
+
             this.context.ais.widgetsManager.update();
             if (connectorDesc.transitionState) {
               this.context.ais.onSearchStateChange(
@@ -129,7 +130,7 @@ export default function createConnector(connectorDesc) {
       componentWillUnmount() {
         this.unsubscribe();
         if (isWidget) {
-          this.unregisterWidget();
+          this.unregisterWidget(); //will schedule an update
           if (hasCleanUp) {
             const newState = connectorDesc.cleanUp.call(
               this,
@@ -140,8 +141,7 @@ export default function createConnector(connectorDesc) {
               ...this.context.ais.store.getState(),
               widgets: newState,
             });
-
-            this.context.ais.onInternalStateUpdate(removeEmptyKey(newState));
+            this.context.ais.onSearchStateChange(removeEmptyKey(newState));
           }
         }
       }

packages/react-instantsearch/src/core/createConnector.test.js

@@ -479,7 +479,7 @@ describe('createConnector', () => {
       })(() => null);
       const unregister = jest.fn();
       const setState = jest.fn();
-      const onInternalStateUpdate = jest.fn();
+      const onSearchStateChange = jest.fn();
       it('unregisters itself on unmount', () => {
         const wrapper = mount(<Connected />, {
           context: {
@@ -492,23 +492,23 @@ describe('createConnector', () => {
               widgetsManager: {
                 registerWidget: () => unregister,
               },
-              onInternalStateUpdate,
+              onSearchStateChange,
             },
           },
         });
         expect(unregister.mock.calls.length).toBe(0);
         expect(setState.mock.calls.length).toBe(0);
-        expect(onInternalStateUpdate.mock.calls.length).toBe(0);
+        expect(onSearchStateChange.mock.calls.length).toBe(0);
 
         wrapper.unmount();
 
         expect(unregister.mock.calls.length).toBe(1);
         expect(setState.mock.calls.length).toBe(1);
-        expect(onInternalStateUpdate.mock.calls.length).toBe(1);
+        expect(onSearchStateChange.mock.calls.length).toBe(1);
         expect(setState.mock.calls[0][0]).toEqual({
           widgets: { another: { state: 'state' } },
         });
-        expect(onInternalStateUpdate.mock.calls[0][0]).toEqual({
+        expect(onSearchStateChange.mock.calls[0][0]).toEqual({
           another: { state: 'state' },
         });
       });
@@ -524,13 +524,13 @@ describe('createConnector', () => {
               widgetsManager: {
                 registerWidget: () => unregister,
               },
-              onInternalStateUpdate,
+              onSearchStateChange,
             },
           },
         });
         wrapper.unmount();
 
-        expect(onInternalStateUpdate.mock.calls[0][0]).toEqual({
+        expect(onSearchStateChange.mock.calls[0][0]).toEqual({
           another: { state: 'state' },
         });
       });

packages/react-instantsearch/src/core/createInstantSearchManager.js

@@ -46,6 +46,12 @@ export default function createInstantSearchManager(
     searching: false,
   });
 
+  let skip = false;
+
+  function skipSearch() {
+    skip = true;
+  }
+
   function updateClient(client) {
     helper.setClient(client);
     search();
@@ -109,35 +115,37 @@ export default function createInstantSearchManager(
   }
 
   function search() {
-    const {
-      sharedParameters,
-      mainIndexParameters,
-      derivatedWidgets,
-    } = getSearchParameters(helper.state);
-
-    Object.values(derivedHelpers).forEach(d => d.detach());
-    derivedHelpers = {};
-
-    helper.setState(sharedParameters);
-
-    derivatedWidgets.forEach(derivatedSearchParameters => {
-      const index = derivatedSearchParameters.targetedIndex;
-      const derivedHelper = helper.derive(() => {
-        const parameters = derivatedSearchParameters.widgets.reduce(
-          (res, widget) => widget.getSearchParameters(res),
-          sharedParameters.setIndex(index)
-        );
-        indexMapping[parameters.index] = index;
-        return parameters;
+    if (!skip) {
+      const {
+        sharedParameters,
+        mainIndexParameters,
+        derivatedWidgets,
+      } = getSearchParameters(helper.state);
+
+      Object.values(derivedHelpers).forEach(d => d.detach());
+      derivedHelpers = {};
+
+      helper.setState(sharedParameters);
+
+      derivatedWidgets.forEach(derivatedSearchParameters => {
+        const index = derivatedSearchParameters.targetedIndex;
+        const derivedHelper = helper.derive(() => {
+          const parameters = derivatedSearchParameters.widgets.reduce(
+            (res, widget) => widget.getSearchParameters(res),
+            sharedParameters.setIndex(index)
+          );
+          indexMapping[parameters.index] = index;
+          return parameters;
+        });
+        derivedHelper.on('result', handleSearchSuccess);
+        derivedHelper.on('error', handleSearchError);
+        derivedHelpers[index] = derivedHelper;
       });
-      derivedHelper.on('result', handleSearchSuccess);
-      derivedHelper.on('error', handleSearchError);
-      derivedHelpers[index] = derivedHelper;
-    });
 
-    helper.setState(mainIndexParameters);
+      helper.setState(mainIndexParameters);
 
-    helper.search();
+      helper.search();
+    }
   }
 
   function handleSearchSuccess(content) {
@@ -274,5 +282,6 @@ export default function createInstantSearchManager(
     onSearchForFacetValues,
     updateClient,
     updateIndex,
+    skipSearch,
   };
 }

packages/react-instantsearch/src/core/createInstantSearchManager.test.js

@@ -175,6 +175,27 @@ describe('createInstantSearchManager', () => {
         expect(client1.search).toHaveBeenCalledTimes(1);
       });
     });
+    it('should not be called when the search is skipped', () => {
+      const client0 = makeClient(defaultResponse);
+      expect(client0.search).toHaveBeenCalledTimes(0);
+      const ism = createInstantSearchManager({
+        indexName: 'index',
+        initialState: {},
+        searchParameters: {},
+        algoliaClient: client0,
+      });
+
+      ism.skipSearch();
+
+      ism.widgetsManager.registerWidget({
+        getMetadata: () => {},
+        transitionState: () => {},
+      });
+
+      return Promise.resolve().then(() => {
+        expect(client0.search).toHaveBeenCalledTimes(0);
+      });
+    });
   });
 });