elastic / search-ui

Search UI. Libraries for the fast development of modern, engaging search experiences.
https://docs.elastic.co/search-ui
Apache License 2.0
1.92k stars 368 forks source link

SearchDriver makes a redundant search request with defaultState during initialization under some edge cases #975

Closed wentaoxu415 closed 1 year ago

wentaoxu415 commented 1 year ago

Describe the bug There is a bug within SearchDriver.ts in the search-ui package that triggers a redundant search request with default state during its initialization when the following conditions are met:

The reason why our team has this initial state is that it helps our application serve facets fast in the initial load by forcing Elasticsearch to use cached results of the aggregation query.

However, our team saw that this specific set of configurations causes a redundant search request that overrides the results of the original search request made with the initial state.

Under this scenario, the first search request gets triggered with the initialState through this _updateSearchResults call in the source file since the alwaysSearchOnInitialLoad is set true.

However, the first search request also causes URL state change which can cause a component managing the SearchDriver to be re-rendered and initialized again. Then in the 2nd initialization, the SearchDriver proceeds to make the 2nd search request through another _updateSearchResults call from this other line in the source file if trackUrlState is also set to true. In that search request, the state being passed is the requestState that is merged between DEFAULT_STATE and urlState. However, if the urLState was empty, this 2nd request gets made with DEFAULT_STATE and this leads to totally different search results than what you expected from your initialState.

To Reproduce Steps to reproduce the behavior:

  1. Configure the <SearchProvider /> component from react-search-ui with the conditions mentioned above (specifically, with initialState: {resultsPerPage: 0} which will eventually pass these conditions to the SearchDriver in search-ui
  2. Load the page with that <SearchProvider />
  3. Instead of loading 0 search results as expected from the initialState, it will load 20 search results which comes from the DEFAULT_STATE constant instead. Moreover, the network tab indicates that it has made 2 search requests.

Expected behavior Only 1 search request should get made and the initial state should be respected, not overridden.

Screenshots N/A

Which backends and packages are you using: Backend: Elasticsearch: 8.7.0 App Search: 8.7.0

Frontend: react-search-ui: 1.20.0 search-ui: 1.20.0

Other Thoughts It seems weird that the SearchDriver class has 2 places where it can potentially make the _updateSearchResults in the initialization (Line 238 and 288) and each of them resolves search state differently.

I think a better solution is for the SearchDriver initialization to have only 1 place where it can make the _updateSearchResults call and the other part of the initialization to build the correct request state based on the configuration.

botelastic[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Is this issue still important to you? If so, please leave a comment and let us know. As always, thank you for your contributions.