Skip to content

Commit

Permalink
fix(InstantSearch): remove useless walk/duplicate request (#4127)
Browse files Browse the repository at this point in the history
**Summary**

This PR removes a useless walk on the indices to apply the `SearchParameters`, it also removes a useless duplicate request (since we walk two times, we call search two times). At the `init` step we walk on the tree of indices to create each instance with their `uiState` and `SearchParameters`. It means that the state is already set, we don't have to trigger another walk.

The extra walk was called with the result of `read` which is not correct. Inside the constructor we already merge both `initialUiState` & `read`. If an `initialUiState` is present with refinements they are all overridden by `read`. With Storybook we mock the router to always return an empty object. It means that we always remove the `initialUiState` (see below).

You can also see the difference for the duplicate request on the e-commerce example. Here is the [link](https://5d81f53123e91200085f41ae--instantsearchjs.netlify.com/examples/e-commerce/) for the version prior to this PR. Open the DevTools you'll notice two identical requests on start. Here is the [link](https://deploy-preview-4127--instantsearchjs.netlify.com/examples/e-commerce/) for the version with this PR. Open the DevTools you'll notice only one request on start.

**Before**

![Screenshot 2019-09-17 at 15 37 36](https://user-images.githubusercontent.com/6513513/65046859-ac764d00-d961-11e9-9b7f-f76dae19fa28.png)

You can check the live version on [Storybook](https://5d81f53123e91200085f41ae--instantsearchjs.netlify.com/stories/?path=/story/currentrefinements--with-refinementlist).

**After**

![Screenshot 2019-09-17 at 15 38 57](https://user-images.githubusercontent.com/6513513/65046865-af713d80-d961-11e9-9c54-ce9c10878aaf.png)

You can check the live version on [Storybook](https://deploy-preview-4127--instantsearchjs.netlify.com/stories/?path=/story/currentrefinements--with-refinementlist).
  • Loading branch information
samouss authored and Haroenv committed Oct 23, 2019
1 parent 569d573 commit 70163a8
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 27 deletions.
1 change: 0 additions & 1 deletion src/lib/InstantSearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,6 @@ See ${createDocumentationLink({
});

if (this._routingManager) {
this._routingManager.applySearchParameters(this._routingManager.read());
this._routingManager.subscribe();
}

Expand Down
45 changes: 20 additions & 25 deletions src/lib/RoutingManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,30 +34,6 @@ class RoutingManager {
this.instantSearchInstance = instantSearchInstance;

this.createURL = this.createURL.bind(this);
this.applySearchParameters = this.applySearchParameters.bind(this);
}

public applySearchParameters(uiState: UiState): void {
walk(this.instantSearchInstance.mainIndex, current => {
const widgets = current.getWidgets();
const indexUiState = uiState[current.getIndexId()] || {};

const searchParameters = widgets.reduce((parameters, widget) => {
if (!widget.getWidgetSearchParameters) {
return parameters;
}

return widget.getWidgetSearchParameters(parameters, {
uiState: indexUiState,
});
}, current.getHelper()!.state);

current
.getHelper()!
.overrideStateWithoutTriggeringChangeEvent(searchParameters);

this.instantSearchInstance.scheduleSearch();
});
}

public read(): UiState {
Expand All @@ -76,7 +52,26 @@ class RoutingManager {
this.router.onUpdate(route => {
const uiState = this.stateMapping.routeToState(route);

this.applySearchParameters(uiState);
walk(this.instantSearchInstance.mainIndex, current => {
const widgets = current.getWidgets();
const indexUiState = uiState[current.getIndexId()] || {};

const searchParameters = widgets.reduce((parameters, widget) => {
if (!widget.getWidgetSearchParameters) {
return parameters;
}

return widget.getWidgetSearchParameters(parameters, {
uiState: indexUiState,
});
}, current.getHelper()!.state);

current
.getHelper()!
.overrideStateWithoutTriggeringChangeEvent(searchParameters);

this.instantSearchInstance.scheduleSearch();
});
});
}

Expand Down
17 changes: 17 additions & 0 deletions src/lib/__tests__/InstantSearch-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,23 @@ describe('start', () => {
expect(widget.init).toHaveBeenCalledTimes(1);
});

it('triggers a single search with `routing`', async () => {
const searchClient = createSearchClient();
const search = new InstantSearch({
indexName: 'indexName',
routing: true,
searchClient,
});

expect(searchClient.search).toHaveBeenCalledTimes(0);

search.start();

await runAllMicroTasks();

expect(searchClient.search).toHaveBeenCalledTimes(1);
});

it('triggers a search without errors', () => {
const searchClient = createSearchClient();
const search = new InstantSearch({
Expand Down
2 changes: 1 addition & 1 deletion src/lib/__tests__/RoutingManager-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ describe('RoutingManager', () => {
search.once('render', () => {
// initialization is done at this point
expect(widget.render).toHaveBeenCalledTimes(1);
expect(widget.getWidgetSearchParameters).toHaveBeenCalledTimes(2);
expect(widget.getWidgetSearchParameters).toHaveBeenCalledTimes(1);

expect(router.write).toHaveBeenCalledTimes(0);

Expand Down

0 comments on commit 70163a8

Please sign in to comment.