enable react-hooks lint rules (#11511)

* enable `react-hooks` lint rules

* Clean up Prettier, Size-limit, and Api-Extractor

* update eslint rule

* different type casting to not break lint rule parsing

* fix up useMutation + lifecycle test

* Clean up Prettier, Size-limit, and Api-Extractor

* avoid reading/writing ref in render

* fix up useLazyQuery

* Clean up Prettier, Size-limit, and Api-Extractor

* fix up another test doing a mutation in render

* more useQuery cleanup

* Clean up Prettier, Size-limit, and Api-Extractor

* ignore rule of hook for context access

* undo changes that were moved into separate PRs

* Clean up Prettier, Size-limit, and Api-Extractor

* almost full rewrite of the `useSubscription` hook

* Clean up Prettier, Size-limit, and Api-Extractor

* ignore any rule violation lint warnings in useSubscription and useQuery

* changeset

* rename patch

* also add "eslint-plugin-react-compiler" rules

---------

Co-authored-by: phryneas <phryneas@users.noreply.github.com>

Author: phryneas

.changeset/famous-camels-rescue.md

@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+`useLoadableQuery`: ensure that `loadQuery` is updated if the ApolloClient instance changes

.eslintrc

@@ -23,10 +23,13 @@
     {
       "files": ["**/*.ts", "**/*.tsx"],
       "excludedFiles": ["**/__tests__/**/*.*", "*.d.ts"],
+      "extends": ["plugin:react-hooks/recommended"],
       "parserOptions": {
         "project": "./tsconfig.json"
       },
+      "plugins": ["eslint-plugin-react-compiler"],
       "rules": {
+        "react-compiler/react-compiler": "error",
         "@typescript-eslint/consistent-type-imports": [
           "error",
           {

package-lock.json

@@ -148,6 +148,8 @@
     "eslint-import-resolver-typescript": "3.6.1",
     "eslint-plugin-import": "npm:@phryneas/eslint-plugin-import@2.27.5-pr.2813.2817.199971c",
     "eslint-plugin-local-rules": "2.0.1",
+    "eslint-plugin-react-compiler": "0.0.0-experimental-c8b3f72-20240517",
+    "eslint-plugin-react-hooks": "4.6.2",
     "eslint-plugin-testing-library": "6.2.2",
     "expect-type": "0.19.0",
     "fetch-mock": "9.11.0",

package.json

@@ -0,0 +1,28 @@
+diff --git a/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js b/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js
+index 441442f..d1ec5dc 100644
+--- a/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js
++++ b/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js
+@@ -905,7 +905,7 @@ var ExhaustiveDeps = {
+         var _callee = callee,
+             name = _callee.name;
+ 
+-        if (name === 'useRef' && id.type === 'Identifier') {
++        if ((name === 'useRef' || name === "useLazyRef") && id.type === 'Identifier') {
+           // useRef() return value is stable.
+           return true;
+         } else if (name === 'useState' || name === 'useReducer') {
+diff --git a/node_modules/eslint-plugin-react-hooks/index.js b/node_modules/eslint-plugin-react-hooks/index.js
+index 0e91baf..7e86d46 100644
+--- a/node_modules/eslint-plugin-react-hooks/index.js
++++ b/node_modules/eslint-plugin-react-hooks/index.js
+@@ -1,9 +1,3 @@
+ 'use strict';
+ 
+-// TODO: this doesn't make sense for an ESLint rule.
+-// We need to fix our build process to not create bundles for "raw" packages like this.
+-if (process.env.NODE_ENV === 'production') {
+-  module.exports = require('./cjs/eslint-plugin-react-hooks.production.min.js');
+-} else {
+-  module.exports = require('./cjs/eslint-plugin-react-hooks.development.js');
+-}
++module.exports = require('./cjs/eslint-plugin-react-hooks.development.js');

patches/eslint-plugin-react-hooks+4.6.2.patch

@@ -12,6 +12,7 @@ Relay does this too, so we hope this is safe.
 https://github.com/facebook/relay/blob/8651fbca19adbfbb79af7a3bc40834d105fd7747/packages/react-relay/relay-hooks/loadQuery.js#L90-L98
 */
 export function useRenderGuard() {
+  // eslint-disable-next-line react-compiler/react-compiler
   RenderDispatcher = getRenderDispatcher();
 
   return React.useCallback(() => {

src/react/hooks/internal/useRenderGuard.ts

@@ -245,12 +245,19 @@ export function useLoadableQuery<
 
       setQueryRef(wrapQueryRef(queryRef));
     },
-    [query, queryKey, suspenseCache, watchQueryOptions, calledDuringRender]
+    [
+      query,
+      queryKey,
+      suspenseCache,
+      watchQueryOptions,
+      calledDuringRender,
+      client,
+    ]
   );
 
   const reset: ResetFunction = React.useCallback(() => {
     setQueryRef(null);
-  }, [queryRef]);
+  }, []);
 
   return [loadQuery, queryRef, { fetchMore, refetch, reset }];
 }

src/react/hooks/useLoadableQuery.ts

@@ -230,13 +230,16 @@ class InternalState<TData, TVariables extends OperationVariables> {
     // initialization, this.renderPromises is usually undefined (unless SSR is
     // happening), but that's fine as long as it has been initialized that way,
     // rather than left uninitialized.
+    // eslint-disable-next-line react-hooks/rules-of-hooks
     this.renderPromises = React.useContext(getApolloContext()).renderPromises;
 
     this.useOptions(options);
 
     const obsQuery = this.useObservableQuery();
 
+    // eslint-disable-next-line react-hooks/rules-of-hooks
     const result = useSyncExternalStore(
+      // eslint-disable-next-line react-hooks/rules-of-hooks
       React.useCallback(
         (handleStoreChange) => {
           if (this.renderPromises) {
@@ -307,7 +310,9 @@ class InternalState<TData, TVariables extends OperationVariables> {
           // effectively passing this dependency array to that useEffect buried
           // inside useSyncExternalStore, as desired.
           obsQuery,
+          // eslint-disable-next-line react-hooks/exhaustive-deps
           this.renderPromises,
+          // eslint-disable-next-line react-hooks/exhaustive-deps
           this.client.disableNetworkFetches,
         ]
       ),
@@ -533,6 +538,7 @@ class InternalState<TData, TVariables extends OperationVariables> {
       this.observable || // Reuse this.observable if possible (and not SSR)
       this.client.watchQuery(this.getObsQueryOptions()));
 
+    // eslint-disable-next-line react-hooks/rules-of-hooks
     this.obsQueryFields = React.useMemo(
       () => ({
         refetch: obsQuery.refetch.bind(obsQuery),

src/react/hooks/useQuery.ts

@@ -59,6 +59,10 @@ export function useQueryRefHandlers<
       // client that's available to us at the current position in the React tree
       // that ApolloClient will then have the job to recreate a real queryRef from
       // the transported object
+      // This is just a context read - it's fine to do this conditionally.
+      // This hook wrapper also shouldn't be optimized by React Compiler.
+      // eslint-disable-next-line react-compiler/react-compiler
+      // eslint-disable-next-line react-hooks/rules-of-hooks
     : useApolloClient()
   )(queryRef);
 }

src/react/hooks/useQueryRefHandlers.ts

@@ -52,6 +52,10 @@ export function useReadQuery<TData>(
       // client that's available to us at the current position in the React tree
       // that ApolloClient will then have the job to recreate a real queryRef from
       // the transported object
+      // This is just a context read - it's fine to do this conditionally.
+      // This hook wrapper also shouldn't be optimized by React Compiler.
+      // eslint-disable-next-line react-compiler/react-compiler
+      // eslint-disable-next-line react-hooks/rules-of-hooks
     : useApolloClient()
   )(queryRef);
 }
@@ -85,7 +89,7 @@ function _useReadQuery<TData>(
           forceUpdate();
         });
       },
-      [internalQueryRef]
+      [internalQueryRef, queryRef]
     ),
     getPromise,
     getPromise

src/react/hooks/useReadQuery.ts

@@ -204,6 +204,8 @@ export function useSubscription<
     }
 
     Object.assign(ref.current, { client, subscription, options });
+    // eslint-disable-next-line react-compiler/react-compiler
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [client, subscription, options, canResetObservableRef.current]);
 
   React.useEffect(() => {
@@ -271,6 +273,8 @@ export function useSubscription<
         subscription.unsubscribe();
       });
     };
+    // eslint-disable-next-line react-compiler/react-compiler
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [observable]);
 
   return result;

src/react/hooks/useSubscription.ts

@@ -251,18 +251,18 @@ function _useSuspenseQuery<
   }, [queryRef.result]);
 
   const result = fetchPolicy === "standby" ? skipResult : __use(promise);
-  const fetchMore = React.useCallback(
-    ((options) => {
+
+  const fetchMore = React.useCallback<
+    FetchMoreFunction<unknown, OperationVariables>
+  >(
+    (options) => {
       const promise = queryRef.fetchMore(options);
       setPromise([queryRef.key, queryRef.promise]);
 
       return promise;
-    }) satisfies FetchMoreFunction<
-      unknown,
-      OperationVariables
-    > as FetchMoreFunction<TData | undefined, TVariables>,
+    },
     [queryRef]
-  );
+  ) as FetchMoreFunction<TData | undefined, TVariables>;
 
   const refetch: RefetchFunction<TData, TVariables> = React.useCallback(
     (variables) => {

src/react/hooks/useSuspenseQuery.ts

@@ -81,6 +81,8 @@ export const useSyncExternalStore: RealUseSESHookType =
           // Force a re-render.
           forceUpdate({ inst });
         }
+        // React Hook React.useLayoutEffect has a missing dependency: 'inst'. Either include it or remove the dependency array.
+        // eslint-disable-next-line react-hooks/exhaustive-deps
       }, [subscribe, value, getSnapshot]);
     } else {
       Object.assign(inst, { value, getSnapshot });
@@ -108,6 +110,8 @@ export const useSyncExternalStore: RealUseSESHookType =
           forceUpdate({ inst });
         }
       });
+      // React Hook React.useEffect has a missing dependency: 'inst'. Either include it or remove the dependency array.
+      // eslint-disable-next-line react-hooks/exhaustive-deps
     }, [subscribe]);
 
     return value;