Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

react-router example issuing multiple queries per search #1834

Closed
joshed-io opened this issue Jan 15, 2017 · 4 comments
Closed

react-router example issuing multiple queries per search #1834

joshed-io opened this issue Jan 15, 2017 · 4 comments
Assignees

Comments

@joshed-io
Copy link
Contributor

Do you want to request a feature or report a bug?
Bug in the react-router example

Bug: What is the current behavior?
In the react-router example, 3 queries are sent to Algolia for each refinement of the search.

Bug: What is the expected behavior?
1 query is sent to Algolia for each refinement.

Bug: What browsers are impacted? Which versions?
All.

To reproduce, load the react-router example inside of this repository and watch the network tab as you add one letter to the search box. You should see 3 calls to Algolia where only 1 is expected. The first search happens as expected from <SearchBox/> widget, the second and third searches happen because the <App/> component re-renders as a result of prop and state changes, from the location change and onSearchStateChange. I believe React-InstantSearch is behaving as expected, to re-trigger the search on each render, but I wasn't able to find a good way to implement shouldComponentUpdate that prevents additional renders but still allows child components to update with the new search state. Thoughts?

What project are you opening an issue for?

  • react-instantsearch

What is the version you are using? Always use the latest one before opening a bug issue.
Latest: 486f005

@mthuret
Copy link
Contributor

mthuret commented Jan 16, 2017

Hi @dzello thanks for reporting this :)

One thing you can already do is adding the shouldComponentUpdate lifecycle function to not render if the searchState is the same between this.state and nextState.

But there's still one other request made and I still don't know why. I'll will investigate more and update the example when found.

@mthuret
Copy link
Contributor

mthuret commented Jan 16, 2017

So in the end there is two things going on here:

  • There's an extra render that occur but which is not due to React InstantSearch. To fix this one, you just need to add:
shouldComponentUpdate(nextProps, nextState) {
    return !isEqual(this.state.searchState, nextState.searchState);
  }
  • When passing new props to widgets we were only checking for a shallow equality. In this case it's not enough because some props are arrays.

You can follow the different fixes in this PR #1840.

@joshed-io
Copy link
Contributor Author

Thanks for the speedy reply @mthuret, I'll follow along over there. 💯

@mthuret
Copy link
Contributor

mthuret commented Jan 19, 2017

I'm closing this since it should be ok with the 2.2.0 of RIS :)

@mthuret mthuret closed this as completed Jan 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants