fix(hooks): prevent infinite loops from render state (#3455)

Author: francoischalifour

packages/react-instantsearch-hooks/package.json

@@ -48,7 +48,6 @@
   "dependencies": {
     "@babel/runtime": "^7.1.2",
     "algoliasearch-helper": "^3.7.4",
-    "dequal": "^2.0.0",
     "instantsearch.js": "^4.40.1"
   },
   "peerDependencies": {

packages/react-instantsearch-hooks/src/hooks/__tests__/useConnector.test.tsx

@@ -8,9 +8,13 @@ import {
   createSearchClient,
   createSingleSearchResponse,
 } from '../../../../../test/mock';
-import { createInstantSearchTestWrapper } from '../../../../../test/utils';
+import {
+  createInstantSearchTestWrapper,
+  wait,
+} from '../../../../../test/utils';
 import { Index } from '../../components/Index';
 import { InstantSearch } from '../../components/InstantSearch';
+import { useHits } from '../../connectors/useHits';
 import { IndexContext } from '../../lib/IndexContext';
 import { InstantSearchContext } from '../../lib/InstantSearchContext';
 import { noop } from '../../lib/noop';
@@ -133,6 +137,56 @@ const connectCustomSearchBoxWithoutRenderState: Connector<
     };
   };
 
+const connectUnstableSearchBox: Connector<
+  CustomSearchBoxWidgetDescription,
+  Record<string, never>
+> =
+  (renderFn, unmountFn = noop) =>
+  (widgetParams) => {
+    return {
+      $$type: 'test.searchBox',
+      init(params) {
+        renderFn(
+          {
+            ...this.getWidgetRenderState!(params),
+            instantSearchInstance: params.instantSearchInstance,
+          },
+          true
+        );
+      },
+      render(params) {
+        renderFn(
+          {
+            ...this.getWidgetRenderState!(params),
+            query: 'query',
+            instantSearchInstance: params.instantSearchInstance,
+          },
+          false
+        );
+      },
+      dispose() {
+        unmountFn();
+      },
+      getWidgetRenderState({ helper, state }) {
+        return {
+          query: state.query || '',
+          // This creates a new reference for `refine()` at every render.
+          refine: (value) => helper.setQuery(value).search(),
+          widgetParams,
+        };
+      },
+      getWidgetUiState(uiState, { searchParameters }) {
+        return {
+          ...uiState,
+          query: searchParameters.query,
+        };
+      },
+      getWidgetSearchParameters(searchParameters, { uiState }) {
+        return searchParameters.setQueryParameter('query', uiState.query || '');
+      },
+    };
+  };
+
 describe('useConnector', () => {
   test('returns the connector render state', async () => {
     const wrapper = createInstantSearchTestWrapper();
@@ -448,4 +502,43 @@ describe('useConnector', () => {
       ])
     );
   });
+
+  test('limits the number of renders with unstable function references from render state', async () => {
+    const searchClient = createSearchClient({});
+
+    function Hits(props) {
+      useHits(props);
+      return null;
+    }
+
+    function Search() {
+      // Use a connector with unstable function references in render state
+      const { query } = useConnector(connectUnstableSearchBox);
+
+      return (
+        <>
+          <input value={query} />
+          {/* Use unstable function as prop */}
+          <Hits transformItems={(items) => items} />
+        </>
+      );
+    }
+
+    function App() {
+      return (
+        <InstantSearch searchClient={searchClient} indexName="indexName">
+          <Search />
+        </InstantSearch>
+      );
+    }
+
+    render(<App />);
+
+    await wait(0);
+
+    // This checks that InstantSearch doesn't re-render endlessly. We should
+    // still be able to optimize this render count to `1`, but `2` is acceptable
+    // for now compared to an infinite loop.
+    expect(searchClient.search).toHaveBeenCalledTimes(2);
+  });
 });

packages/react-instantsearch-hooks/src/hooks/useConnector.ts

@@ -1,6 +1,7 @@
 import { useMemo, useRef, useState } from 'react';
 
 import { createSearchResults } from '../lib/createSearchResults';
+import { dequal } from '../lib/dequal';
 import { useIndexContext } from '../lib/useIndexContext';
 import { useInstantSearchContext } from '../lib/useInstantSearchContext';
 import { useInstantSearchServerContext } from '../lib/useInstantSearchServerContext';
@@ -27,6 +28,9 @@ export function useConnector<
     additionalWidgetProperties
   );
   const shouldSetStateRef = useRef(true);
+  const previousRenderStateRef = useRef<TDescription['renderState'] | null>(
+    null
+  );
 
   const widget = useMemo(() => {
     const createWidget = connector(
@@ -54,8 +58,22 @@ export function useConnector<
           const { instantSearchInstance, widgetParams, ...renderState } =
             connectorState;
 
-          // eslint-disable-next-line @typescript-eslint/no-use-before-define
-          setState(renderState);
+          // We only update the state when a widget render state param changes,
+          // except for functions. We ignore function reference changes to avoid
+          // infinite loops. It's safe to omit them because they get updated
+          // every time another render param changes.
+          if (
+            !dequal(
+              renderState,
+              previousRenderStateRef.current,
+              (a, b) =>
+                a?.constructor === Function && b?.constructor === Function
+            )
+          ) {
+            // eslint-disable-next-line @typescript-eslint/no-use-before-define
+            setState(renderState);
+            previousRenderStateRef.current = renderState;
+          }
         }
       },
       () => {

packages/react-instantsearch-hooks/src/lib/__tests__/dequal.test.ts

@@ -0,0 +1,35 @@
+import { dequal } from '../dequal';
+
+describe('dequal', () => {
+  const areFunctions = (a: any, b: any): boolean =>
+    a?.constructor === Function && b?.constructor === Function;
+
+  test('without compare returns false for functions', () => {
+    expect(
+      dequal(
+        () => {},
+        () => {}
+      )
+    ).toEqual(false);
+  });
+
+  test('with a compare returns true for functions', () => {
+    expect(
+      dequal(
+        () => {},
+        () => {},
+        areFunctions
+      )
+    ).toEqual(true);
+  });
+
+  test('without compare returns false for nested functions', () => {
+    expect(dequal({ fn: () => {} }, { fn: () => {} })).toEqual(false);
+  });
+
+  test('with a compare returns true for nested functions', () => {
+    expect(dequal({ fn: () => {} }, { fn: () => {} }, areFunctions)).toEqual(
+      true
+    );
+  });
+});

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

@@ -0,0 +1,54 @@
+/* eslint-disable complexity */
+
+/*
+ * Code taken from dequal/lite v2.0.0
+ * https://github.com/lukeed/dequal/blob/9aa73181ac7e081cd330cac67d313632ac04bb02/src/lite.js
+ *
+ * It adds a 3rd argument `compare(a, b)` that lets execute custom logic to
+ * compare values.
+ * We use it to skip comparing function references.
+ */
+
+const has = Object.prototype.hasOwnProperty;
+
+export function dequal(
+  foo: any,
+  bar: any,
+  compare?: (a: any, b: any) => boolean
+) {
+  // start of custom implementation
+  if (compare?.(foo, bar)) {
+    return true;
+  }
+  // end of custom implementation
+
+  let ctor;
+  let len;
+  if (foo === bar) return true;
+
+  if (foo && bar && (ctor = foo.constructor) === bar.constructor) {
+    if (ctor === Date) return foo.getTime() === bar.getTime();
+    if (ctor === RegExp) return foo.toString() === bar.toString();
+
+    if (ctor === Array) {
+      if ((len = foo.length) === bar.length) {
+        while (len-- && dequal(foo[len], bar[len], compare));
+      }
+      return len === -1;
+    }
+
+    if (!ctor || typeof foo === 'object') {
+      len = 0;
+      // eslint-disable-next-line guard-for-in, no-restricted-syntax
+      for (ctor in foo) {
+        if (has.call(foo, ctor) && ++len && !has.call(bar, ctor)) return false;
+        if (!(ctor in bar) || !dequal(foo[ctor], bar[ctor], compare))
+          return false;
+      }
+      return Object.keys(bar).length === len;
+    }
+  }
+
+  // eslint-disable-next-line no-self-compare
+  return foo !== foo && bar !== bar;
+}

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

@@ -1,6 +1,7 @@
-import { dequal } from 'dequal/lite';
 import { useEffect, useState } from 'react';
 
+import { dequal } from '../lib/dequal';
+
 export function useStableValue<TValue>(value: TValue) {
   const [stableValue, setStableValue] = useState<TValue>(() => value);
 

yarn.lock

@@ -12524,11 +12524,6 @@ deprecation@^2.0.0, deprecation@^2.3.1:
   resolved "https://registry.yarnpkg.com/deprecation/-/deprecation-2.3.1.tgz#6368cbdb40abf3373b525ac87e4a260c3a700919"
   integrity sha512-xmHIy4F3scKVwMsQ4WnVaS8bHOx0DmVwRywosKhaILI0ywMDWPtBSku2HNxRvF7jtwDRsoEwYQSfbxj8b7RlJQ==
 
-dequal@^2.0.0:
-  version "2.0.2"
-  resolved "https://registry.yarnpkg.com/dequal/-/dequal-2.0.2.tgz#85ca22025e3a87e65ef75a7a437b35284a7e319d"
-  integrity sha512-q9K8BlJVxK7hQYqa6XISGmBZbtQQWVXSrRrWreHC94rMt1QL/Impruc+7p2CYSYuVIUr+YCt6hjrs1kkdJRTug==
-
 des.js@^1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/des.js/-/des.js-1.0.0.tgz#c074d2e2aa6a8a9a07dbd61f9a15c2cd83ec8ecc"