fix(hooks): prevent widget cleanup on <InstantSearch> unmount (#3590)

Author: FabienMotte

.eslintrc.js

@@ -89,6 +89,12 @@ const config = {
         paths: ['instantsearch.js/es'],
       },
     ],
+    'react-hooks/exhaustive-deps': [
+      'warn',
+      {
+        additionalHooks: '(useIsomorphicLayoutEffect)',
+      },
+    ],
   },
   overrides: [
     {

packages/react-instantsearch-hooks/global.d.ts

@@ -1 +1,8 @@
 declare const __DEV__: boolean;
+
+import 'instantsearch.js';
+declare module 'instantsearch.js' {
+  declare class InstantSearch {
+    _preventWidgetCleanup?: boolean;
+  }
+}

packages/react-instantsearch-hooks/src/components/__tests__/InstantSearch.test.tsx

@@ -6,7 +6,7 @@ import React, { StrictMode, Suspense, version as ReactVersion } from 'react';
 import { SearchBox } from 'react-instantsearch-hooks-web';
 
 import { createSearchClient } from '../../../../../test/mock';
-import { createInstantSearchSpy } from '../../../../../test/utils';
+import { createInstantSearchSpy, wait } from '../../../../../test/utils';
 import { useRefinementList } from '../../connectors/useRefinementList';
 import version from '../../version';
 import { Index } from '../Index';
@@ -749,4 +749,55 @@ describe('InstantSearch', () => {
       expect(searchFunction2).toHaveBeenCalledTimes(5);
     });
   });
+
+  test('triggers no search on unmount', async () => {
+    const searchClient = createSearchClient({});
+
+    function App() {
+      return (
+        <StrictMode>
+          <InstantSearch searchClient={searchClient} indexName="indexName">
+            <SearchBox />
+            <RefinementList attribute="brand" />
+            <RefinementList attribute="price" />
+          </InstantSearch>
+        </StrictMode>
+      );
+    }
+
+    const { unmount } = render(<App />);
+
+    await waitFor(() => expect(searchClient.search).toHaveBeenCalledTimes(1));
+
+    unmount();
+
+    // We need to wait for the `cleanup` functions of the instance and
+    // the widgets to get called since they are schedule with a `setTimeout`.
+    await wait(100);
+
+    expect(searchClient.search).toHaveBeenCalledTimes(1);
+  });
+
+  test('triggers a search on widget unmount', async () => {
+    const searchClient = createSearchClient({});
+
+    function App({ isMounted }) {
+      return (
+        <StrictMode>
+          <InstantSearch searchClient={searchClient} indexName="indexName">
+            <SearchBox />
+            {isMounted && <RefinementList attribute="brand" />}
+          </InstantSearch>
+        </StrictMode>
+      );
+    }
+
+    const { rerender } = render(<App isMounted={true} />);
+
+    await waitFor(() => expect(searchClient.search).toHaveBeenCalledTimes(1));
+
+    rerender(<App isMounted={false} />);
+
+    await waitFor(() => expect(searchClient.search).toHaveBeenCalledTimes(2));
+  });
 });

packages/react-instantsearch-hooks/src/lib/useInstantSearchApi.ts

@@ -138,19 +138,12 @@ export function useInstantSearchApi<TUiState extends UiState, TRouteState>(
         // We cancel the previous cleanup function because we don't want to
         // dispose the search during an update.
         clearTimeout(cleanupTimerRef.current);
+        search._preventWidgetCleanup = false;
       }
 
       return () => {
         function cleanup() {
-          // When `dispose()` is called, InstantSearch.js also calls `removeWigets()`.
-          // This is not desired because React starts by unmounting each widget
-          // before unmounting <InstantSearch>, so their `dispose()` methods are
-          // already called.
-          // Calling `removeWidgets()` would remove each widget multiple times.
-          const originalRemoveWidgets = search.removeWidgets;
-          search.removeWidgets = () => search;
           search.dispose();
-          search.removeWidgets = originalRemoveWidgets;
         }
 
         // We clean up only when the component that uses this subscription unmounts,
@@ -159,7 +152,12 @@ export function useInstantSearchApi<TUiState extends UiState, TRouteState>(
         // Executing the cleanup function in a `setTimeout()` lets us cancel it
         // in the next effect.
         // (There might be better ways to do this.)
-        cleanupTimerRef.current = setTimeout(cleanup, 0);
+        cleanupTimerRef.current = setTimeout(cleanup);
+
+        // We need to prevent the `useWidget` cleanup function so that widgets
+        // are not removed before the instance is disposed, triggering
+        // an unwanted search request.
+        search._preventWidgetCleanup = true;
       };
     }, [forceUpdate]),
     () => searchRef.current,

packages/react-instantsearch-hooks/src/lib/useWidget.ts

@@ -1,6 +1,7 @@
 import { useEffect, useRef } from 'react';
 
 import { dequal } from './dequal';
+import { useInstantSearchContext } from './useInstantSearchContext';
 import { useIsomorphicLayoutEffect } from './useIsomorphicLayoutEffect';
 
 import type { Widget } from 'instantsearch.js';
@@ -31,13 +32,16 @@ export function useWidget<TWidget extends Widget | IndexWidget, TProps>({
   const shouldAddWidgetEarly =
     shouldSsr && !parentIndex.getWidgets().includes(widget);
 
+  const search = useInstantSearchContext();
+
   // This effect is responsible for adding, removing, and updating the widget.
   // We need to support scenarios where the widget is remounted quickly, like in
   // Strict Mode, so that we don't lose its state, and therefore that we don't
   // break routing.
   useIsomorphicLayoutEffect(() => {
     const previousWidget = prevWidgetRef.current;
     function cleanup() {
+      if (search._preventWidgetCleanup) return;
       parentIndex.removeWidgets([previousWidget]);
     }
 
@@ -76,7 +80,7 @@ export function useWidget<TWidget extends Widget | IndexWidget, TProps>({
       // we're able to cancel it in the next effect.
       cleanupTimerRef.current = setTimeout(cleanup);
     };
-  }, [parentIndex, widget, shouldAddWidgetEarly]);
+  }, [parentIndex, widget, shouldAddWidgetEarly, search, props]);
 
   if (shouldAddWidgetEarly) {
     parentIndex.addWidgets([widget]);