Tao Chen
Tao Chen

Reputation: 305

How to let react consider returned value of custom hook as stable identity?

In the react document, we can find:

Note

React guarantees that setState function identity is stable and won’t change on re-renders. This is why it’s safe to omit from the useEffect or useCallback dependency list.

Sometimes, custom hook can also guarantee the returned function identity is stable, is it possible to let react know it?


Added after discussing with Jayce444:

If react dose not consider the returned value of custom hook as stable identity but we omit it from the dependency list of other hooks, npm will report warnings

Upvotes: 9

Views: 2180

Answers (3)

Fancy John
Fancy John

Reputation: 39428

Considering you want to silence react-hooks/exhaustive-deps eslint rule for specific names, I have written a patch for that rule, which allows me to use the following config:

'react-hooks/exhaustive-deps': [
  'warn',
  {
    stableHooksPattern: 'useDispatch|useSharedValue|useErrorDropdown',
    ignoredDepsPattern: '^navigation|navigate|popToTop|pop$',
  },
],

which essentially results in the following hook call being "clean":

const navigation = useNavigation();

useEffect(() => {
  navigation.navigate('Voila');
}, []);

Here's the source code of my patch for [email protected]:

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 29fb123..b22bc3f 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
@@ -735,6 +735,12 @@ var ExhaustiveDeps = {
         },
         enableDangerousAutofixThisMayCauseInfiniteLoops: {
           type: 'boolean'
+        },
+        stableHooksPattern: {
+          type: 'string'
+        },
+        ignoredDepsPattern: {
+          type: 'string'
         }
       }
     }]
@@ -743,9 +749,13 @@ var ExhaustiveDeps = {
     // Parse the `additionalHooks` regex.
     var additionalHooks = context.options && context.options[0] && context.options[0].additionalHooks ? new RegExp(context.options[0].additionalHooks) : undefined;
     var enableDangerousAutofixThisMayCauseInfiniteLoops = context.options && context.options[0] && context.options[0].enableDangerousAutofixThisMayCauseInfiniteLoops || false;
+    var stableHooksPattern = context.options && context.options[0] && context.options[0].stableHooksPattern ? new RegExp(context.options[0].stableHooksPattern) : undefined;
+    var ignoredDepsPattern = context.options && context.options[0] && context.options[0].ignoredDepsPattern ? new RegExp(context.options[0].ignoredDepsPattern) : undefined;
     var options = {
       additionalHooks: additionalHooks,
-      enableDangerousAutofixThisMayCauseInfiniteLoops: enableDangerousAutofixThisMayCauseInfiniteLoops
+      enableDangerousAutofixThisMayCauseInfiniteLoops: enableDangerousAutofixThisMayCauseInfiniteLoops,
+      stableHooksPattern: stableHooksPattern,
+      ignoredDepsPattern: ignoredDepsPattern
     };
 
     function reportProblem(problem) {
@@ -904,7 +914,9 @@ var ExhaustiveDeps = {
         var _callee = callee,
             name = _callee.name;
 
-        if (name === 'useRef' && id.type === 'Identifier') {
+        if (options.stableHooksPattern && options.stableHooksPattern.test(name)) {
+          return true;
+        } else if (name === 'useRef' && id.type === 'Identifier') {
           // useRef() return value is stable.
           return true;
         } else if (name === 'useState' || name === 'useReducer') {
@@ -1098,7 +1110,7 @@ var ExhaustiveDeps = {
 
             if (!dependencies.has(dependency)) {
               var resolved = reference.resolved;
-              var isStable = memoizedIsStablecKnownHookValue(resolved) || memoizedIsFunctionWithoutCapturedValues(resolved);
+              var isStable = memoizedIsStablecKnownHookValue(resolved) || memoizedIsFunctionWithoutCapturedValues(resolved) || (options.ignoredDepsPattern && options.ignoredDepsPattern.test(resolved.name));
               dependencies.set(dependency, {
                 isStable: isStable,
                 references: [reference]

The filename for the patch is eslint-plugin-react-hooks+4.3.0.patch, and you should place it into patches directory at the root of your repository.

Disclaimer: this will only work globally. So if you want to declare a return value as "stable", you will have to list it in the eslint config, and it will take effect globally in your project.

If you want to be able to "mark" return values as stable, you will need to further modify that eslint plugin to make it work. Unfortunately, I don't have time for that.

Upvotes: 2

Tao Chen
Tao Chen

Reputation: 305

@Jayce444, Thanks very much, I know your option.

My goal is to declare the hook like useState very much, ThisHook = useState + immer(https://github.com/immerjs/immer)

Here is my custom hook

import produce, { Draft } from "immer";
import { useCallback, useState } from "react";

export type DraftMutation<T> = (draft: Draft<T>) => void;

export function useImmerState<T>(
    initialValue: T
): [T, (mutation: DraftMutation<T>) => void] {
    const [value, setValue] = useState(initialValue);
    const setValueByImmer = useCallback((mutation: DraftMutation<T>) => {
        setValue(oldValue => {
            return produce(oldValue, draft => mutation(draft));
        });
    }, []);
    return [value, setValueByImmer];
}

Then, let's discuss how to use it.

Setp 1, define a simple type, like this:

interface Point { 
    readonly x: number;
    readonly y: number;
}

Setp 2, use my custom hook is functional component

const [point, setPoint] = useImmerState<Point>({x: 0, y: 0});
const onButtonClick = useCallback(() => {
    setPoint(draft => {
        draft.x++;
        draft.y++;
    });
}, []); //Need not add 'setPoint' into the dependency list, and no eslint warining should appear

Your demo is very greate, but this hook can guarantee it have no problems. This hook looks like useState very much, that's why I want this future. React should support it if the developer knowns what he/she is doing.

Upvotes: 2

Jayce444
Jayce444

Reputation: 9073

In your case you don't really want to just hide that warning for custom code. React does it for setState functions because it's referring to something inside its own library. As commenters have mentioned, you can disable the linting rule for that specific line, but it's probably better just to include this dependency.

When you write code, you generally want it to be loosely coupled from its context, to make no assumptions about where exactly it's being used. While in your current use case, you know the function from the hook isn't changing, that could change in the future. Consider this example:

const useCustomHook = () => {
    const calculate = useCallback((number) => {
        // Do stuff here
    }, []);
    return ({ calculate });
};


const MyComponent = () => {
    const [number, setNumber] = useState(0);
    const { calculate } = useCustomHook();

    useEffect(() => {
        calculate(number);
    }, [number]);

    // rest of the component
};

Simple example, you have a memoized calculate function returned from the custom hook, and a number in your component's state. When the number changes, recalculate. And you can see, we've left calculate out of the useEffect dependencies, as you're wanting to do in your use case.

But let's say this changes, and we replace the custom hook with this one:

const useCustomHook = () => {
    const someValue = useContext(someRandomContext);

    const calculateOne = (number) => {/* some code */};
    const calculateTwo = (number) => {/* some code */};

    const calculate = useCallback(someValue ? calculateOne : calculateTwo, [someValue]);

    return ({ calculate });
};

Now, when the context value changes, the calculate function changes too. However, if you don't have that dependency in your component's useEffect, the calculation won't actually get fired, and you'll now have a stale/incorrect value in your state.

While technically having that dependency may be redundant for you at the moment, if you program defensively you'll avoid bugs like that which can be a pain in the ass to track down. Especially because of the chains of dependencies you can get when using custom hooks, and hooks that use other hooks, etc.. It's literally a handful of extra characters in your dependency array, best to just add it and avoid a potential headache down the road.

Upvotes: 2

Related Questions