algolia / instantsearch

⚡️ Libraries for building performant and instant search and recommend experiences with Algolia. Compatible with JavaScript, TypeScript, React and Vue.
https://www.algolia.com/doc/guides/building-search-ui/what-is-instantsearch/js/
MIT License
3.73k stars 525 forks source link

Dynamically rendering Index components from arrays results in incorrect teardown on unmount #6323

Closed ikesau closed 2 months ago

ikesau commented 3 months ago

Hello. My name's Ike 🙂

Context

I'm trying to build a UI that allows users to preview multiple facets of an index simultaneously.

My mental model for it is a folder structure, where 4 results from each folder are displayed, and nesting into a folder will show 4 results from each subfolder.

The way I've tried doing this is with a separate <Index> and <Configure> pair for each folder. The folder structure is dynamic, so I've tried rendering them using inline array mapping:

const Folder = (props: {name: string}) => (
  <Index indexName="my_index">
    <Configure facetFilters={[`tags:${props.name}`]} hitsPerPage={4} />
    <Hits />
  </Index>
)

['A', 'B'].map(folderName => <Folder key={name} name={folderName} />

Problem

When I update the array, the original folders' widgets aren't disposed correctly, and an erroneous POST is sent, requesting results for both the old folders and the new ones. Then a second POST is sent, for the correct data.

I discovered this doesn't happen when I hardcode the folders, but given that data is dynamic, hardcoding isn't really an option.

Video demo: https://github.com/user-attachments/assets/934f288f-f752-44a8-9829-7fe8c8f0ba57

For discoverabilty, I'll also include the two error messages I get on my local dev environment (though they don't occur in the sandbox)

Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.
Configure
Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.
Hits2

🔍 Steps to reproduce

Render an Index and Configure component by mapping through an array, then update the array so that the Index and Configure component is configured differently. Look at your network requests and see that 2 requests have been sent. One containing the old queries+the new queries, and another just containing the new queries.

Live reproduction

https://codesandbox.io/p/sandbox/react-instantsearch-array-cleanup-example-wjv8j3?file=%2Fsrc%2FApp.tsx

💭 Expected behavior

Updating the array responsible for the rendering of Index components shouldn't result in 2 queries being sent. Only 1 query, for the correct data should be sent.

Package version

algoliasearch 4.23.2, instantsearch.js 4.72.1, react-instantsearch 7.12.2

Operating system

macOs 14.5

Browser

Firefox 130

Code of Conduct

ikesau commented 3 months ago

I investigated this some more today and it seems to be due to a race condition that occurs between the index.addWidgets and index.dispose calls. It doesn't always happen, but the more <Index> components that are being rendered, the more likely it is.

ikesau commented 2 months ago

I've avoided the issue by building my own client using the Algolia REST API instead. Fixing this race condition seemed to rely on some pretty deep react/instantsearch internals and I couldn't imagine being able to solve that as an outsider without introducing other regressions.

Closing the issue 🙂