react-instantsearch icon indicating copy to clipboard operation
react-instantsearch copied to clipboard

Using `<SortBy />` component breaks Algolia widget logic when combined with several index sections.

Open jsancho opened this issue 2 years ago • 3 comments

🐛 Bug description

Using <SortBy /> component breaks Algolia widget logic when combined with several indexes across sections/pages.

🔍 Bug reproduction

Steps to reproduce the behavior:

We are using multiple indexes on a single app, and we store the current index in the local state. So that whenever the user goes to a different section of the app, the data is retrieved form the corresponding index.

<InstantSearch indexName={indexName} searchClient={searchClient}>
    <Hits section={indexName} />
</InstantSearch>

This is largely working fine and we have added search and facet filtering/refinement lists.

The issue occurs after adding a SortBy widget and transitioning from one section to another. The initial network call seems to be fine, as it's requesting data from the section we have just navigated to.

However, there's something odd with the render cycles, as the data on the view is stale. Also, if we follow up with filtering a facet, the request will be issued to the previous index.

If we disable the SortBy widget altogether, everything else works as expected, including search box and facet filtering across page transitions.

Live reproduction: On the sandbox below start with sorting disabled and you will see how you can show data from both indexes without problem. Enable some filters, transition from one section to another to see how facets changes and the totals are updated.

https://codesandbox.io/s/agolia-multiple-indexes-without-routing-svgob3?file=/src/App.tsx

Once you enable SortBy with the checkbox up top, filtering and querying will be generally broken as described above.

Interestingly, if you select an actual sort other than "relevance" the UI will start refreshing and it will all work again.

Meaning that to be able to use the SortBy widget with page transitions the user has to select an actual sort value. Otherwise, the UI will break as described.

💭 Expected behavior

To be able to combine SortBy with the other widgets that we are using.

Environment

  • React InstantSearch Hooks version: 6.30.3
  • React version: 18.0.0
  • Browser: latest
  • OS: windows

jsancho avatar Aug 04 '22 12:08 jsancho

Hello @jsancho!

What's currently happening is that when you update the prop indexName on <InstantSearch>, it re-renders the tree, then the <SortBy> widget is unmounted and remounted. In the connectSortBy connector in InstantSearch.js, in the dispose method, we reset the index to the initial one. This setIndex is overriding the one on the instance, so nothing is happening visually.

I've forked your sandbox and used the <Configure> widget instead to update the current index. The indexName prop on <InstantSearch> is initialized with the default index. The rest is the same.

https://codesandbox.io/s/agolia-multiple-indexes-without-routing-fixed-l7bqip?file=/src/App.tsx

Hope that helps!

FabienMotte avatar Aug 10 '22 13:08 FabienMotte

Hi @jsancho!

Did the provided Sandbox solve the issue on your end so I can close it? Thank you!

FabienMotte avatar Aug 29 '22 11:08 FabienMotte

Hi @FabienMotte,

This has definitely helped with our setup. I've not replied, because the we were experiencing other issues that I wasn't entirely sure that they were related to this particular setting.

Matter of fact is that our final version of the code uses the same value for both the <InstantSearch indexName={current_index}> component and the <Configure index={current_index}> one.

Which leaves me a bit confused on how should we be using this API and if we might have a sub-optimal configuration. I would have maybe expected for <InstantSearch> to only be concerned with the client configuration and for the index prop to be a concern of <Configuration> exclusively.

We've been trying many different combinations of setting the index value along with the routing, when refreshing the page, navigating from a deep link, etc, to some mixed results.

I suppose that we could close the issue and, if necessary, I'll open another one with more context on what we are trying to do.

Come to think of it, I'd say that the best clarification would be to see an example on what would be the recommended best practice to follow when making an app that uses multiple indexes that change along with the active route.

Hope this is making sense somewhat :/

What are your thoughts?

jsancho avatar Aug 29 '22 12:08 jsancho

Hey @jsancho,

Sorry for the late reply, we're doing a round of cleanup before migrating this repository to the new InstantSearch monorepo. This issue is to be mostly solved, so we're going to close it, feel free to open a new one if needed.

FabienMotte avatar Dec 22 '22 15:12 FabienMotte