From 3234b1204d7f33861a2aba84209579efa1ad0efa Mon Sep 17 00:00:00 2001
From: Maxime Janton <127086@supinfo.com>
Date: Mon, 27 Nov 2017 18:36:28 +0100
Subject: [PATCH] fix(InstantSearch.dispose): dont call `getConfiguration` of
 URLSync widget (#2604)

---
 src/lib/InstantSearch.js                | 14 ++++----
 src/lib/__tests__/InstantSearch-test.js | 46 ++++++++++++++++++++++---
 2 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/src/lib/InstantSearch.js b/src/lib/InstantSearch.js
index a5c8017b4f..8ba89e77a2 100644
--- a/src/lib/InstantSearch.js
+++ b/src/lib/InstantSearch.js
@@ -181,9 +181,11 @@ Usage: instantsearch({
       // re-compute remaining widgets to the state
       // in a case two widgets were using the same configuration but we removed one
       if (nextState) {
-        this.searchParameters = this.widgets.reduce(enhanceConfiguration({}), {
-          ...nextState,
-        });
+        // We dont want to re-add URlSync `getConfiguration` widget
+        // it can throw errors since it may re-add SearchParameters about something unmounted
+        this.searchParameters = this.widgets
+          .filter(w => w.constructor.name !== 'URLSync')
+          .reduce(enhanceConfiguration({}), { ...nextState });
 
         this.helper.setState(this.searchParameters);
       }
@@ -272,11 +274,7 @@ Usage: instantsearch({
    * @return {undefined} This method does not return anything
    */
   dispose() {
-    this.removeWidgets(
-      this.widgets
-        .slice()
-        .sort(widget => (widget.constructor.name === 'URLSync' ? -1 : 1))
-    );
+    this.removeWidgets(this.widgets);
   }
 
   createURL(params) {
diff --git a/src/lib/__tests__/InstantSearch-test.js b/src/lib/__tests__/InstantSearch-test.js
index fc27f3cb92..a27e411cc2 100644
--- a/src/lib/__tests__/InstantSearch-test.js
+++ b/src/lib/__tests__/InstantSearch-test.js
@@ -369,12 +369,12 @@ describe('InstantSearch lifecycle', () => {
         facets: ['categories'],
         maxValuesPerFacet: 10,
       },
-      dispose = sinon.spy()
+      dispose = jest.fn()
     ) {
       const widget = {
-        getConfiguration: sinon.stub().returns(widgetGetConfiguration),
-        init: sinon.spy(),
-        render: sinon.spy(),
+        getConfiguration: jest.fn(() => widgetGetConfiguration),
+        init: jest.fn(),
+        render: jest.fn(),
         dispose,
       };
 
@@ -526,6 +526,44 @@ describe('InstantSearch lifecycle', () => {
       expect(search.searchParameters.numericRefinements).toEqual({});
       expect(search.searchParameters.disjunctiveFacets).toEqual([]);
     });
+
+    it('should unmount a widget without calling URLSync wiget getConfiguration', () => {
+      // fake url-sync widget
+      const spy = jest.fn();
+
+      class URLSync {
+        constructor() {
+          this.getConfiguration = spy;
+          this.init = jest.fn();
+          this.render = jest.fn();
+          this.dispose = jest.fn();
+        }
+      }
+
+      const urlSyncWidget = new URLSync();
+      expect(urlSyncWidget.constructor.name).toEqual('URLSync');
+
+      search.addWidget(urlSyncWidget);
+
+      // add fake widget to dispose
+      // that returns a `nextState` while dispose
+      const widget1 = registerWidget(
+        undefined,
+        jest.fn(({ state: nextState }) => nextState)
+      );
+
+      const widget2 = registerWidget();
+      search.start();
+
+      // remove widget1
+      search.removeWidget(widget1);
+
+      // it should have been called only once after start();
+      expect(spy).toHaveBeenCalledTimes(1);
+
+      // but widget2 getConfiguration() should have been called twice
+      expect(widget2.getConfiguration).toHaveBeenCalledTimes(2);
+    });
   });
 
   describe('When adding widgets after start', () => {