Fix stable identity of functions in hooks #66 (#67)

packages/mst-query/package.json

@@ -24,15 +24,15 @@
     ],
     "devDependencies": {
         "@testing-library/react": "^13.4.0",
-        "@types/react": "^17.0.3",
+        "@types/react": "^18.3.3",
         "jsdom": "^20.0.1",
         "microbundle": "^0.15.0",
         "mobx": "6.6.1",
         "mobx-react": "7.5.2",
         "mobx-state-tree": "5.1.5",
         "prettier": "^2.2.1",
-        "react": "^18.2.0",
-        "react-dom": "^18.2.0",
+        "react": "^18.3.1",
+        "react-dom": "^18.3.1",
         "typescript": "^4.6.4",
         "vitest": "^0.24.3"
     },

packages/mst-query/src/MstQueryHandler.ts

@@ -320,19 +320,14 @@ export class MstQueryHandler {
     invalidate() {
         const isFetched = this.isFetched;
 
-        const refetchQuery = async () => {
-            const next = await this.refetch();
-            next();
-        };
-
         this.cachedAt = undefined;
 
         if (isFetched) {
             this.isFetched = false;
         }
 
         if (isFetched && this.queryObservers.length > 0) {
-            refetchQuery();
+            this.model.refetch();
         }
     }
 

packages/mst-query/src/hooks.ts

@@ -1,10 +1,11 @@
 import { getSnapshot, getType, Instance, isStateTreeNode, SnapshotIn } from 'mobx-state-tree';
-import { useContext, useEffect, useRef, useState } from 'react';
+import { useCallback, useContext, useEffect, useRef, useState } from 'react';
 import { VolatileQuery, MutationReturnType, QueryReturnType } from './create';
 import { Context } from './QueryClientProvider';
 import { QueryClient } from './QueryClient';
 import { EmptyPagination, EmptyRequest, QueryObserver } from './MstQueryHandler';
 import equal from '@wry/equality';
+import { useEvent } from './utils';
 
 function mergeWithDefaultOptions(key: string, options: any, queryClient: QueryClient<any>) {
     return Object.assign({ queryClient }, (queryClient.config as any)[key], {
@@ -113,13 +114,15 @@ export function useMutation<T extends Instance<MutationReturnType>>(
         mutation,
     };
 
-    const mutate = <TResult = any>(params: {
-        request: SnapshotIn<T['variables']['request']>;
-        optimisticUpdate?: () => void;
-    }) => {
-        const result = mutation.mutate({ ...params, ...options } as any);
-        return result as Promise<{ data: T['data']; error: any; result: TResult }>;
-    };
+    const mutate = useEvent(
+        <TResult = any>(params: {
+            request: SnapshotIn<T['variables']['request']>;
+            optimisticUpdate?: () => void;
+        }) => {
+            const result = mutation.mutate({ ...params, ...options } as any);
+            return result as Promise<{ data: T['data']; error: any; result: TResult }>;
+        }
+    );
 
     return [mutate, result] as [typeof mutate, typeof result];
 }

packages/mst-query/src/utils.ts

@@ -8,6 +8,7 @@ import {
     isFrozenType,
 } from 'mobx-state-tree';
 import { isObservableArray } from 'mobx';
+import * as React from 'react';
 
 export function getRealTypeFromObject(typeDef: any, data: any, key: any) {
     const modelOrBaseType = getSubType(typeDef, data[key]);
@@ -56,3 +57,52 @@ function isArray(a: any) {
 export function isObject(a: any) {
     return a && typeof a === 'object' && !isArray(a);
 }
+
+// From https://github.com/scottrippey/react-use-event-hook
+
+type AnyFunction = (...args: any[]) => any;
+
+/**
+ * Suppress the warning when using useLayoutEffect with SSR. (https://reactjs.org/link/uselayouteffect-ssr)
+ * Make use of useInsertionEffect if available.
+ */
+const useInsertionEffect =
+    typeof window !== 'undefined'
+        ? // useInsertionEffect is available in React 18+
+          React.useInsertionEffect || React.useLayoutEffect
+        : () => {};
+
+/**
+ * Similar to useCallback, with a few subtle differences:
+ * - The returned function is a stable reference, and will always be the same between renders
+ * - No dependency lists required
+ * - Properties or state accessed within the callback will always be "current"
+ */
+export function useEvent<TCallback extends AnyFunction>(callback: TCallback): TCallback {
+    // Keep track of the latest callback:
+    const latestRef = React.useRef<TCallback>(useEvent_shouldNotBeInvokedBeforeMount as any);
+    useInsertionEffect(() => {
+        latestRef.current = callback;
+    }, [callback]);
+
+    // Create a stable callback that always calls the latest callback:
+    // using useRef instead of useCallback avoids creating and empty array on every render
+    const stableRef = React.useRef<TCallback>(null as any);
+    if (!stableRef.current) {
+        stableRef.current = function (this: any) {
+            return latestRef.current.apply(this, arguments as any);
+        } as TCallback;
+    }
+
+    return stableRef.current;
+}
+
+/**
+ * Render methods should be pure, especially when concurrency is used,
+ * so we will throw this error if the callback is called while rendering.
+ */
+function useEvent_shouldNotBeInvokedBeforeMount() {
+    throw new Error(
+        'INVALID_USEEVENT_INVOCATION: the callback from useEvent cannot be invoked before the component has mounted.'
+    );
+}

packages/mst-query/tests/mstQuery.test.tsx

@@ -964,3 +964,26 @@ test('invalidate', async () => {
 
     configureMobx({ enforceActions: 'observed' });
 });
+
+test('stable identity for hook callbacks', async () => {
+    const { render, q } = setup();
+
+    const runSideEffect = vi.fn();
+
+    const Comp = observer(() => {
+        const [add] = useMutation(q.addItemMutation);
+        React.useEffect(() => {
+            runSideEffect();
+        }, [add]);
+        return <div></div>;
+    });
+
+    render(<Comp />);
+    await wait(0);
+
+    q.addItemMutation.mutate({ request: { message: 'test', path: 'test' } });
+
+    await wait(0);
+
+    expect(runSideEffect).toHaveBeenCalledTimes(1);
+});