[hooks] Don't use GetId functions in handler memo

Fixes #152

Author: jamesgpearce

src/ui-react/hooks.ts

@@ -212,7 +212,12 @@ import type {
   ResultTableCellIdsListener,
   ResultTableListener,
 } from '../@types/queries/index.d.ts';
-import {arrayIsEmpty, arrayIsEqual, arrayMap} from '../common/array.ts';
+import {
+  arrayFilter,
+  arrayIsEmpty,
+  arrayIsEqual,
+  arrayMap,
+} from '../common/array.ts';
 import {
   getUndefined,
   ifNotUndefined,
@@ -392,8 +397,15 @@ const useSetCallback = <Parameter, Thing>(
           ),
         ),
       ),
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    [store, settable, ...getDeps, ...thenDeps, ...args],
+    /* eslint-disable react-hooks/exhaustive-deps */
+    [
+      store,
+      settable,
+      ...getDeps,
+      ...thenDeps,
+      ...arrayFilter(args, (arg) => !isFunction(arg)),
+    ],
+    /* eslint-enable react-hooks/exhaustive-deps */
   );
 };
 

test/unit/ui-react/hooks.test.tsx

@@ -2741,30 +2741,39 @@ describe('Write Hooks', () => {
     expect(clickHandler1).not.toEqual(clickHandler2);
   });
 
-  test('useSetTableCallback', () => {
+  test('useSetTableCallback (including handler memo)', () => {
     const then = jest.fn((_store?: Store, _table?: Table) => null);
+    let previousHandler: any;
+    let handlerChanged = 0;
     const Test = ({
       value,
       then,
     }: {
       readonly value: number;
       readonly then: (store?: Store, table?: Table) => void;
     }) => {
-      return (
-        <div
-          onClick={useSetTableCallback<React.MouseEvent<HTMLDivElement>>(
-            't1',
-            (e) => ({r1: {c1: e.screenX * value}}),
-            [value],
-            store,
-            then,
-          )}
-        />
+      const handler = useSetTableCallback<React.MouseEvent<HTMLDivElement>>(
+        't1',
+        (e) => ({r1: {c1: e.screenX * value}}),
+        [value],
+        store,
+        then,
       );
+      if (handler != previousHandler) {
+        handlerChanged++;
+        previousHandler = handler;
+      }
+      return <div onClick={handler} />;
     };
     act(() => {
       renderer = create(<Test value={2} then={then} />);
     });
+    expect(handlerChanged).toEqual(1);
+
+    act(() => {
+      renderer.update(<Test value={2} then={then} />);
+    });
+    expect(handlerChanged).toEqual(1);
 
     const clickHandler1 = renderer.root.findByType('div').props.onClick;
     act(() => {
@@ -2776,34 +2785,44 @@ describe('Write Hooks', () => {
     act(() => {
       renderer.update(<Test value={3} then={then} />);
     });
+    expect(handlerChanged).toEqual(2);
     const clickHandler2 = renderer.root.findByType('div').props.onClick;
     expect(clickHandler1).not.toEqual(clickHandler2);
   });
 
-  test('useSetTableCallback, parameterized Id', () => {
+  test('useSetTableCallback, parameterized Id (including handler memo)', () => {
     const then = jest.fn((_store?: Store, _table?: Table) => null);
+    let previousHandler: any;
+    let handlerChanged = 0;
     const Test = ({
       value,
       then,
     }: {
       readonly value: number;
       readonly then: (store?: Store, table?: Table) => void;
     }) => {
-      return (
-        <div
-          onClick={useSetTableCallback<React.MouseEvent<HTMLDivElement>>(
-            (e) => 't' + e.screenY,
-            (e) => ({r1: {c1: e.screenX * value}}),
-            [value],
-            store,
-            then,
-          )}
-        />
+      const handler = useSetTableCallback<React.MouseEvent<HTMLDivElement>>(
+        (e) => 't' + e.screenY,
+        (e) => ({r1: {c1: e.screenX * value}}),
+        [value],
+        store,
+        then,
       );
+      if (handler != previousHandler) {
+        handlerChanged++;
+        previousHandler = handler;
+      }
+      return <div onClick={handler} />;
     };
     act(() => {
       renderer = create(<Test value={2} then={then} />);
     });
+    expect(handlerChanged).toEqual(1);
+
+    act(() => {
+      renderer.update(<Test value={2} then={then} />);
+    });
+    expect(handlerChanged).toEqual(1);
 
     const clickHandler1 = renderer.root.findByType('div').props.onClick;
     act(() => {
@@ -2821,6 +2840,7 @@ describe('Write Hooks', () => {
     act(() => {
       renderer.update(<Test value={3} then={then} />);
     });
+    expect(handlerChanged).toEqual(2);
     const clickHandler2 = renderer.root.findByType('div').props.onClick;
     expect(clickHandler1).not.toEqual(clickHandler2);
   });