From 70163a837e4bc1cae29c10942cc3d784acc03531 Mon Sep 17 00:00:00 2001 From: Samuel Vaillant Date: Wed, 18 Sep 2019 14:32:25 +0200 Subject: [PATCH] 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). --- src/lib/InstantSearch.ts | 1 - src/lib/RoutingManager.ts | 45 +++++++++++------------- src/lib/__tests__/InstantSearch-test.js | 17 +++++++++ src/lib/__tests__/RoutingManager-test.ts | 2 +- 4 files changed, 38 insertions(+), 27 deletions(-) diff --git a/src/lib/InstantSearch.ts b/src/lib/InstantSearch.ts index c93de6f55c..1214dd5bf7 100644 --- a/src/lib/InstantSearch.ts +++ b/src/lib/InstantSearch.ts @@ -413,7 +413,6 @@ See ${createDocumentationLink({ }); if (this._routingManager) { - this._routingManager.applySearchParameters(this._routingManager.read()); this._routingManager.subscribe(); } diff --git a/src/lib/RoutingManager.ts b/src/lib/RoutingManager.ts index e88ee242a1..314568f6ea 100644 --- a/src/lib/RoutingManager.ts +++ b/src/lib/RoutingManager.ts @@ -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 { @@ -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(); + }); }); } diff --git a/src/lib/__tests__/InstantSearch-test.js b/src/lib/__tests__/InstantSearch-test.js index d368690944..82d26e53b8 100644 --- a/src/lib/__tests__/InstantSearch-test.js +++ b/src/lib/__tests__/InstantSearch-test.js @@ -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({ diff --git a/src/lib/__tests__/RoutingManager-test.ts b/src/lib/__tests__/RoutingManager-test.ts index 2d729ec4fe..195c8dbd41 100644 --- a/src/lib/__tests__/RoutingManager-test.ts +++ b/src/lib/__tests__/RoutingManager-test.ts @@ -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);