Skip to content

Commit 70163a8

Browse files
samoussHaroenv
authored andcommitted
fix(InstantSearch): remove useless walk/duplicate request (#4127)
**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).
1 parent 569d573 commit 70163a8

File tree

4 files changed

+38
-27
lines changed

4 files changed

+38
-27
lines changed

src/lib/InstantSearch.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,6 @@ See ${createDocumentationLink({
413413
});
414414

415415
if (this._routingManager) {
416-
this._routingManager.applySearchParameters(this._routingManager.read());
417416
this._routingManager.subscribe();
418417
}
419418

src/lib/RoutingManager.ts

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -34,30 +34,6 @@ class RoutingManager {
3434
this.instantSearchInstance = instantSearchInstance;
3535

3636
this.createURL = this.createURL.bind(this);
37-
this.applySearchParameters = this.applySearchParameters.bind(this);
38-
}
39-
40-
public applySearchParameters(uiState: UiState): void {
41-
walk(this.instantSearchInstance.mainIndex, current => {
42-
const widgets = current.getWidgets();
43-
const indexUiState = uiState[current.getIndexId()] || {};
44-
45-
const searchParameters = widgets.reduce((parameters, widget) => {
46-
if (!widget.getWidgetSearchParameters) {
47-
return parameters;
48-
}
49-
50-
return widget.getWidgetSearchParameters(parameters, {
51-
uiState: indexUiState,
52-
});
53-
}, current.getHelper()!.state);
54-
55-
current
56-
.getHelper()!
57-
.overrideStateWithoutTriggeringChangeEvent(searchParameters);
58-
59-
this.instantSearchInstance.scheduleSearch();
60-
});
6137
}
6238

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

79-
this.applySearchParameters(uiState);
55+
walk(this.instantSearchInstance.mainIndex, current => {
56+
const widgets = current.getWidgets();
57+
const indexUiState = uiState[current.getIndexId()] || {};
58+
59+
const searchParameters = widgets.reduce((parameters, widget) => {
60+
if (!widget.getWidgetSearchParameters) {
61+
return parameters;
62+
}
63+
64+
return widget.getWidgetSearchParameters(parameters, {
65+
uiState: indexUiState,
66+
});
67+
}, current.getHelper()!.state);
68+
69+
current
70+
.getHelper()!
71+
.overrideStateWithoutTriggeringChangeEvent(searchParameters);
72+
73+
this.instantSearchInstance.scheduleSearch();
74+
});
8075
});
8176
}
8277

src/lib/__tests__/InstantSearch-test.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,23 @@ describe('start', () => {
488488
expect(widget.init).toHaveBeenCalledTimes(1);
489489
});
490490

491+
it('triggers a single search with `routing`', async () => {
492+
const searchClient = createSearchClient();
493+
const search = new InstantSearch({
494+
indexName: 'indexName',
495+
routing: true,
496+
searchClient,
497+
});
498+
499+
expect(searchClient.search).toHaveBeenCalledTimes(0);
500+
501+
search.start();
502+
503+
await runAllMicroTasks();
504+
505+
expect(searchClient.search).toHaveBeenCalledTimes(1);
506+
});
507+
491508
it('triggers a search without errors', () => {
492509
const searchClient = createSearchClient();
493510
const search = new InstantSearch({

src/lib/__tests__/RoutingManager-test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ describe('RoutingManager', () => {
139139
search.once('render', () => {
140140
// initialization is done at this point
141141
expect(widget.render).toHaveBeenCalledTimes(1);
142-
expect(widget.getWidgetSearchParameters).toHaveBeenCalledTimes(2);
142+
expect(widget.getWidgetSearchParameters).toHaveBeenCalledTimes(1);
143143

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

0 commit comments

Comments
 (0)