Skip to content

Commit

Permalink
fix(InstantSearch.dispose): dont call getConfiguration of URLSync w…
Browse files Browse the repository at this point in the history
…idget (#2604)
  • Loading branch information
Maxime Janton authored and bobylito committed Nov 27, 2017
1 parent fff6322 commit 3234b12
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 12 deletions.
14 changes: 6 additions & 8 deletions src/lib/InstantSearch.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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) {
Expand Down
46 changes: 42 additions & 4 deletions src/lib/__tests__/InstantSearch-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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', () => {
Expand Down

0 comments on commit 3234b12

Please sign in to comment.