5

This is part of my code in React js:

export default function Registration() {
    const [email, setEmail] = useState(null);
    const [password, setPassword] = useState(null);
    const [passwordRepeat, setPasswordRepeat] = useState(null);
    const [isFieldsOK, setFieldsOK] = useState(false);

    useEffect(() => {
        if (checkFieldsOK()) {
            setFieldsOK(true);
        } else {
            setFieldsOK(false);
        }
    }, [checkFieldsOK])

    const checkFieldsOK = () => {
        return (isEmail(email) && isValidPassword(password) && passwordRepeat === password);
    }
}

I have the isFieldsOK state which tells me if my fields are valid, and I want it to "listen" to every change in the Registration function. After running this, I get this warning:

The 'checkFieldsOK' function makes the dependencies of useEffect Hook (at line 34) change on every render. Move it inside the useEffect callback. Alternatively, wrap the definition of 'checkFieldsOK' in its own useCallback() Hook  react-hooks/exhaustive-deps

What exactly wrong with my code? What should I change and why? Thank you!

Naor
  • 337
  • 1
  • 3
  • 11
  • If you are using eslint checkout this link. https://stackoverflow.com/questions/59611822/definition-for-rule-react-hooks-exhaustive-deps-was-not-found – A Webb Jan 10 '21 at 00:46

2 Answers2

1

Firstly I would consider moving checkFieldsOK expression above the useEffect.

Then ask yourself, when checkFieldsOK is created, called?

Assuming isEmail and isValidPassword are functions declared outside of your component, to fix the linter issue you need to either wrap the checkFieldsOK with a useCallback:

const checkFieldsOK = useCallback(() => {
        return (
          isEmail(email) &&
          isValidPassword(password) &&
          passwordRepeat === password
        );
    }, [email, password, passwordRepeat])

This way the checkFieldsOK will update it's inputs whenever the email, password or passwordRepeat would change.

or move the checkFieldsOK into the useEffect and update the effect dependencies:

    useEffect(() => {

      const checkFieldsOK = () => {
        return (
          isEmail(email) && 
          isValidPassword(password) && 
          passwordRepeat === password
         );
    }
        if (checkFieldsOK()) {
            setFieldsOK(true);
        } else {
            setFieldsOK(false);
        }
    }, [email, password, passwordRepeat])

This way your effect will be triggered only when email, password or passwordRepeat state changes.

You could simplify further into:

    useEffect(() => {

      const fieldsOK = 
          isEmail(email) && 
          isValidPassword(password) && 
          passwordRepeat === password
        
       setFieldsOK(fieldsOK);

    }, [email, password, passwordRepeat])
mtx
  • 1,196
  • 2
  • 17
  • 41
0

Because you didn't wrap checkFieldsOK in useCallback it's not memoized, which means it gets recreated on every render. And because your useEffect has checkFieldsOK as a dependency, that effect will also run every render.

Exactly as the error message says, first (and I believe incorrect) option is to move the function declaration inside the useEffect like so:

useEffect(() => {
    const checkFieldsOK = () => {
        return (isEmail(email) && isValidPassword(password) && passwordRepeat === password);
    }

    if (checkFieldsOK()) {
        setFieldsOK(true);
    } else {
        setFieldsOK(false);
    }
}, []);

But this means it will only run on mount (since the dependency array is empty), so a better approach is to wrap the checkFieldsOK function in a useCallback so it isn't remade every re-render and the effect only gets run when the field values change:

const checkFieldsOK = useCallback(() => {
    return (isEmail(email) && isValidPassword(password) && passwordRepeat === password);
}, [email, password, passwordRepeat]);

In the second useCallback approach, it behaves like so:

  1. Field value changes
  2. checkFieldsOK function gets remade
  3. useEffect gets run and checks validation of the fields, updating state as needed
Jayce444
  • 8,725
  • 3
  • 27
  • 43