Skip to content

Commit

Permalink
feat(InstantSearch): switch to DerivedHelper only (#3885)
Browse files Browse the repository at this point in the history
  • Loading branch information
@samouss @Haroenv
samouss authored and Haroenv committed Oct 23, 2019
1 parent c7a5571 commit d6fc317
Show file tree
Hide file tree
Showing 2 changed files with 132 additions and 42 deletions.
70 changes: 44 additions & 26 deletions src/lib/InstantSearch.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ See ${createDocumentationLink({
this.client = searchClient;
this.insightsClient = insightsClient;
this.helper = null;
this.derivedHelper = null;
this.indexName = indexName;
this.widgets = [];
this.templatesConfig = {
Expand Down Expand Up @@ -332,6 +333,16 @@ See: https://www.algolia.com/doc/guides/building-search-ui/widgets/create-your-o
initialSearchParameters
);

const derivedHelper = helper.derive(() => helper.state);

helper.search = () => {
// This solution allows us to keep the exact same API for the users but
// under the hood, we have a different implementation. It should be
// completely transparent for the rest of the codebase. Only this module
// is impacted.
helper.searchOnlyWithDerivedHelpers();
};

if (this._searchFunction) {
this._mainHelperSearch = helper.search.bind(helper);
helper.search = () => {
Expand All @@ -351,14 +362,17 @@ See: https://www.algolia.com/doc/guides/building-search-ui/widgets/create-your-o
}

this.helper = helper;
this.derivedHelper = derivedHelper;

this._init(helper.state, this.helper);
this._init(helper, helper.state);

this.helper.on('result', ({ results, state }) => {
this._render(this.helper, results, state);
derivedHelper.on('result', ({ results, state }) => {
this._render(helper, results, state);
});

this.helper.on('error', ({ error }) => {
// Only the "main" Helper emits the `error` event vs the one for `search`
// and `results` that are also emitted on the derived one.
helper.on('error', ({ error }) => {
this.emit('error', {
error,
});
Expand All @@ -367,16 +381,16 @@ See: https://www.algolia.com/doc/guides/building-search-ui/widgets/create-your-o
this._searchStalledTimer = null;
this._isSearchStalled = true;

this.helper.search();
helper.search();

this.helper.on('search', () => {
derivedHelper.on('search', () => {
if (!this._isSearchStalled && !this._searchStalledTimer) {
this._searchStalledTimer = setTimeout(() => {
this._isSearchStalled = true;
this._render(
this.helper,
this.helper.lastResults,
this.helper.lastResults._state
helper,
derivedHelper.lastResults,
derivedHelper.lastResults._state
);
}, this._stalledSearchDelay);
}
Expand All @@ -395,10 +409,14 @@ See: https://www.algolia.com/doc/guides/building-search-ui/widgets/create-your-o
*/
dispose() {
this.removeWidgets(this.widgets);
// You can not start an instance two times, therefore a disposed instance needs to set started as false
// otherwise this can not be restarted at a later point.
// You can not start an instance two times, therefore a disposed instance
// needs to set started as false otherwise this can not be restarted at a
// later point.
this.started = false;

this.derivedHelper.detach();
this.derivedHelper = null;

// The helper needs to be reset to perform the next search from a fresh state.
// If not reset, it would use the state stored before calling `dispose()`.
this.helper.removeAllListeners();
Expand All @@ -414,6 +432,21 @@ See: https://www.algolia.com/doc/guides/building-search-ui/widgets/create-your-o
return this._createURL(this.helper.state.setQueryParameters(params));
}

_init(helper, state) {
this.widgets.forEach(widget => {
if (widget.init) {
widget.init({
state,
helper,
templatesConfig: this.templatesConfig,
createURL: this._createAbsoluteURL,
onHistoryChange: this._onHistoryChange,
instantSearchInstance: this,
});
}
});
}

_render(helper, results, state) {
if (!this.helper.hasPendingRequests()) {
clearTimeout(this._searchStalledTimer);
Expand Down Expand Up @@ -445,21 +478,6 @@ See: https://www.algolia.com/doc/guides/building-search-ui/widgets/create-your-o
*/
this.emit('render');
}

_init(state, helper) {
this.widgets.forEach(widget => {
if (widget.init) {
widget.init({
state,
helper,
templatesConfig: this.templatesConfig,
createURL: this._createAbsoluteURL,
onHistoryChange: this._onHistoryChange,
instantSearchInstance: this,
});
}
});
}
}

export default InstantSearch;
104 changes: 88 additions & 16 deletions src/lib/__tests__/InstantSearch-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ jest.mock('algoliasearch-helper', () => {
const helper = module(...args);

const mock = jest.fn();
helper.search = mock;
helper.searchOnlyWithDerivedHelpers = mock;

return helper;
});
Expand Down Expand Up @@ -320,7 +320,9 @@ See https://www.algolia.com/doc/api-reference/widgets/configure/js/`);
});

it('calls helper.search()', () => {
expect(search.helper.search).toHaveBeenCalledTimes(1);
expect(
search.helper.searchOnlyWithDerivedHelpers
).toHaveBeenCalledTimes(1);
});

it('calls widget.init(helper.state, helper, templatesConfig)', () => {
Expand All @@ -342,7 +344,7 @@ See https://www.algolia.com/doc/api-reference/widgets/configure/js/`);

beforeEach(() => {
results = { some: 'data' };
search.helper.emit('result', {
search.derivedHelper.emit('result', {
state: search.helper.state,
results,
});
Expand Down Expand Up @@ -415,15 +417,15 @@ See https://www.algolia.com/doc/api-reference/widgets/configure/js/`);
expect(render).toHaveBeenCalledTimes(0);
expect(onRender).toHaveBeenCalledTimes(0);

search.helper.emit('result', {
search.derivedHelper.emit('result', {
results: null,
state: search.helper.state,
});

expect(render).toHaveBeenCalledTimes(5);
expect(onRender).toHaveBeenCalledTimes(1);

search.helper.emit('result', {
search.derivedHelper.emit('result', {
results: null,
state: search.helper.state,
});
Expand All @@ -439,15 +441,15 @@ See https://www.algolia.com/doc/api-reference/widgets/configure/js/`);
expect(render).toHaveBeenCalledTimes(0);
expect(onRender).toHaveBeenCalledTimes(0);

search.helper.emit('result', {
search.derivedHelper.emit('result', {
results: null,
state: search.helper.state,
});

expect(render).toHaveBeenCalledTimes(5);
expect(onRender).toHaveBeenCalledTimes(1);

search.helper.emit('result', {
search.derivedHelper.emit('result', {
results: null,
state: search.helper.state,
});
Expand Down Expand Up @@ -663,22 +665,28 @@ See https://www.algolia.com/doc/api-reference/widgets/configure/js/`);

it('should add widgets after start', () => {
search.start();
expect(search.helper.search).toHaveBeenCalledTimes(1);

expect(search.helper.searchOnlyWithDerivedHelpers).toHaveBeenCalledTimes(
1
);
expect(search.widgets).toHaveLength(0);
expect(search.started).toBe(true);

const widget1 = registerWidget({ facets: ['price'] });
search.addWidget(widget1);

expect(search.helper.search).toHaveBeenCalledTimes(2);
expect(search.helper.searchOnlyWithDerivedHelpers).toHaveBeenCalledTimes(
2
);
expect(widget1.init).toHaveBeenCalledTimes(1);

const widget2 = registerWidget({ disjunctiveFacets: ['categories'] });
search.addWidget(widget2);

expect(widget2.init).toHaveBeenCalledTimes(1);
expect(search.helper.search).toHaveBeenCalledTimes(3);
expect(search.helper.searchOnlyWithDerivedHelpers).toHaveBeenCalledTimes(
3
);

expect(search.widgets).toHaveLength(2);
expect(search.helper.state.facets).toEqual(['price']);
Expand All @@ -688,7 +696,9 @@ See https://www.algolia.com/doc/api-reference/widgets/configure/js/`);
it('should trigger only one search using `addWidgets()`', () => {
search.start();

expect(search.helper.search).toHaveBeenCalledTimes(1);
expect(search.helper.searchOnlyWithDerivedHelpers).toHaveBeenCalledTimes(
1
);
expect(search.widgets).toHaveLength(0);
expect(search.started).toBe(true);

Expand All @@ -697,21 +707,27 @@ See https://www.algolia.com/doc/api-reference/widgets/configure/js/`);

search.addWidgets([widget1, widget2]);

expect(search.helper.search).toHaveBeenCalledTimes(2);
expect(search.helper.searchOnlyWithDerivedHelpers).toHaveBeenCalledTimes(
2
);
expect(search.helper.state.facets).toEqual(['price']);
expect(search.helper.state.disjunctiveFacets).toEqual(['categories']);
});

it('should not trigger a search without widgets to add', () => {
search.start();

expect(search.helper.search).toHaveBeenCalledTimes(1);
expect(search.helper.searchOnlyWithDerivedHelpers).toHaveBeenCalledTimes(
1
);
expect(search.widgets).toHaveLength(0);
expect(search.started).toBe(true);

search.addWidgets([]);

expect(search.helper.search).toHaveBeenCalledTimes(1);
expect(search.helper.searchOnlyWithDerivedHelpers).toHaveBeenCalledTimes(
1
);
expect(search.widgets).toHaveLength(0);
expect(search.started).toBe(true);
});
Expand All @@ -734,7 +750,7 @@ See https://www.algolia.com/doc/api-reference/widgets/configure/js/`);
search.start();

expect(search.widgets).toHaveLength(5);
expect(search.helper.search).toHaveBeenCalledTimes(1);
expect(search.helper.searchOnlyWithDerivedHelpers).toHaveBeenCalledTimes(1);
expect(search.started).toBe(true);

// Calling `dispose()` deletes the reference of the `helper`
Expand All @@ -747,7 +763,7 @@ See https://www.algolia.com/doc/api-reference/widgets/configure/js/`);
// called we have to wait for the timeout. Not the best solution but it
// works.
expect(search.widgets).toHaveLength(0);
expect(helper.search).toHaveBeenCalledTimes(1);
expect(helper.searchOnlyWithDerivedHelpers).toHaveBeenCalledTimes(1);
expect(search.started).toBe(false);
done();
}, 100);
Expand Down Expand Up @@ -810,6 +826,62 @@ describe('dispose', () => {

done();
});

it('should set the DerivedHelper to `null`', () => {
const search = new InstantSearch({
indexName: '',
searchClient: {
search() {
return Promise.resolve({
results: [
{
query: 'fake query',
},
],
});
},
},
});

search.start();

expect(search.derivedHelper).not.toBe(null);

search.dispose();

expect(search.derivedHelper).toBe(null);

search.start();

expect(search.derivedHelper).not.toBe(null);
});

it('should detach the DerivedHelper', () => {
const search = new InstantSearch({
indexName: '',
searchClient: {
search() {
return Promise.resolve({
results: [
{
query: 'fake query',
},
],
});
},
},
});

search.start();

expect(search.helper.derivedHelpers).toHaveLength(1);

const { helper } = search;

search.dispose();

expect(helper.derivedHelpers).toHaveLength(0);
});
});

it('Allows to start without widgets', () => {
Expand Down

0 comments on commit d6fc317

Please sign in to comment.