react icon indicating copy to clipboard operation
react copied to clipboard

[React 19] React compiler is warning on mutating values in refs that are used in JSX.

Open NickBlow opened this issue 1 year ago • 5 comments

Summary

When mutating a ref value in an event handler (e.g. when dealing with uncontrolled inputs), the React Compiler ESLint rule is giving the following error:

ESLint: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX(react-compiler/react-compiler)

Linked CodeSandbox:

https://codesandbox.io/p/sandbox/kind-mirzakhani-qzw4t3

Linked Compiler Playground:

https://playground.react.dev/#N4Igzg9grgTgxgUxALhAgHgBwjALgAgBMEAzAQygBsCSoA7OXASwjvwFkBPAQU0wAoAlPmAAdNvjiswNJpQQBJOpii4ASqXwBefFDAINJfAB58ACQAq7ADJKVuAKLyAtgjoEAPvjpVK+AHz4-D6UlIIA3OLi+JLSBAAWZHSE8gDKUABGzkwEOvwIAG5uuMj4GmSMAHQAYjjODkXuxpY2tTD1LsX+wlqBYhIxhcWVmDBD7gAipBTUQpF00TH4TEb8JHKKyqqGlXCwY+74AGRH+Ovydtuku-vD5whgwv1LS1J0Mmcb2p8XW+rXexgB1wlXuYAA2gAGAC68xeMTekHklUoEAA5msNhFFi97pd-iQbkDhgUyJQoAhvqIQNS4UsAL6LenzRZjXCwNj8HEmEh1fCsdJZHJaYCJZJpTLZXD0-zcmLGJh-fC4TiYBBaan3an4MYkEV4v6Gen4AD0soGS2MGVUuFYytV6upYElOWp-kFUuMJutuFtdHNLy9vPaAfw2LojLoIHpQA

This feels slightly wrong to me as the docs specifically call out manipulating the DOM with a ref as a pattern: https://react.dev/reference/react/useRef#manipulating-the-dom-with-a-ref

I guess this is because you can do unsafe changes to refs but might be better off as a warning rather than an error?

NickBlow avatar May 16 '24 13:05 NickBlow

To resolve this error while following the React guidelines, you should ensure that you handle ref mutations in a way that is safe and predictable. In this case, moving the mutation out of the JSX block can help.

` const handleSubmit = (event: React.FormEvent<HTMLFormElement>) => { event.preventDefault();

const input = fileInputRef.current;
if (input && input.files) {
  const file = input.files[0];
  console.log(file);

  // Clear the input value
  input.value = "";
}

};`

HoseinKhanBeigi avatar May 16 '24 14:05 HoseinKhanBeigi

this is the original code

export default function MyApp() {
  const fileInputRef = useRef < HTMLInputElement | null > (null);

  const handleSubmit = (event: React.FormEvent<HTMLFormElement>) => {
    event.preventDefault();

    if (fileInputRef.current && fileInputRef.current.files) {
      const file = fileInputRef.current.files[0];
      console.log(file);
      fileInputRef.current.value = ""; // InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX. Found mutation of `fileInputRef` (10:10)
    }
  };

  return (
    <form onSubmit={handleSubmit}>
      <input type="file" ref={fileInputRef} />
      <button type="submit">Submit</button>
    </form>
  );
}

this gets rid of the error

export default function MyApp() {
  const fileInputRef = useRef < HTMLInputElement | null > (null);

  const inputRef = fileInputRef.current;
  const handleSubmit = (event: React.FormEvent<HTMLFormElement>) => {
    event.preventDefault();

    if (inputRef && inputRef.files) {
      const file = inputRef.files[0];
      console.log(file);
      inputRef.value = "";
    }
  };

  return (
    <form onSubmit={handleSubmit}>
      <input type="file" ref={fileInputRef} />
      <button type="submit">Submit</button>
    </form>
  );
}

but this is also an error:

export default function MyApp() {
  const fileInputRef = useRef < HTMLInputElement | null > (null);

  const inputRef = fileInputRef.current;
  const handleSubmit = (event: React.FormEvent<HTMLFormElement>) => {
    event.preventDefault();

    if (inputRef && inputRef.files) {
      const file = inputRef.files[0];
      console.log(file);
      inputRef.value = ""; // InvalidReact: This mutates a variable that React considers immutable. Found mutation of `inputRef` (11:11)
    }
  };

  const handleBlur = () => {
    handleSubmit();
  }

  return (
    <form onSubmit={handleSubmit}>
      <input type="file" ref={fileInputRef} onBlur={handleBlur}/>
      <button type="submit">Submit</button>
    </form>
  );
}

playground

codingedgar avatar May 16 '24 19:05 codingedgar

This is my best workaround: group dependent functions in an IIFE like a useMemo,

export default function MyApp() {
  const fileInputRef = useRef < HTMLInputElement | null > (null);
  const inputRef = fileInputRef;

  const { handleBlur, handleSubmit } = (() => {
    const handleSubmit2 = (event: React.FormEvent<HTMLFormElement>) => {
      event.preventDefault();

      if (inputRef.current && inputRef.current.files) {
        const file = inputRef.current.files[0];
        console.log(file);
        inputRef.current.value = "";
      }
    };

    return {
      handleBlur: () => {
        handleSubmit2();
      },
      handleSubmit: handleSubmit2
    }
  })()


  return (
    <form onSubmit={handleSubmit}>
      <input type="file" ref={fileInputRef} onBlur={handleBlur} />
      <button type="submit">Submit</button>
    </form>
  );
}

playground

which is kinda weird because the compiler does what one would think perfectly and removes the IIFE

function MyApp() {
  const $ = _c(13);

  const fileInputRef = useRef(null);
  const inputRef = fileInputRef;
  let t0;
  let t1;

  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
    t1 = (event) => {
      event.preventDefault();

      if (inputRef.current && inputRef.current.files) {
        const file = inputRef.current.files[0];
        console.log(file);
        inputRef.current.value = "";
      }
    };

    $[0] = t1;
  } else {
    t1 = $[0];
  }

  const handleSubmit2 = t1;
  let t2;

  if ($[1] !== handleSubmit2) {
    t2 = () => {
      handleSubmit2();
    };

    $[1] = handleSubmit2;
    $[2] = t2;
  } else {
    t2 = $[2];
  }

  let t3;

  if ($[3] !== t2 || $[4] !== handleSubmit2) {
    t3 = {
      handleBlur: t2,
      handleSubmit: handleSubmit2,
    };
    $[3] = t2;
    $[4] = handleSubmit2;
    $[5] = t3;
  } else {
    t3 = $[5];
  }

  t0 = t3;
  const { handleBlur, handleSubmit } = t0;
  let t4;

  if ($[6] !== fileInputRef || $[7] !== handleBlur) {
    t4 = <input type="file" ref={fileInputRef} onBlur={handleBlur} />;
    $[6] = fileInputRef;
    $[7] = handleBlur;
    $[8] = t4;
  } else {
    t4 = $[8];
  }

  let t5;

  if ($[9] === Symbol.for("react.memo_cache_sentinel")) {
    t5 = <button type="submit">Submit</button>;
    $[9] = t5;
  } else {
    t5 = $[9];
  }

  let t6;

  if ($[10] !== handleSubmit || $[11] !== t4) {
    t6 = (
      <form onSubmit={handleSubmit}>
        {t4}
        {t5}
      </form>
    );
    $[10] = handleSubmit;
    $[11] = t4;
    $[12] = t6;
  } else {
    t6 = $[12];
  }

  return t6;
}

Still think refs are a special case that shouldn't be tracked as normal props.

The first iteration I made used const inputRef = fileInputRef.current; but that would be a bug. Thankfully , const inputRef = fileInputRef; is enough to work.

codingedgar avatar May 16 '24 22:05 codingedgar

I believe I have a similar issue that I can't really wrap my head around:

Playground 1

Extracting ref value in function body seems to make it go away, but I have no clue how I could avoid "This mutates a variable that React considers immutable" issue 🫣 :

Playground 2

Edit: Oooooooo

Playground 3

wojtekmaj avatar May 17 '24 10:05 wojtekmaj

Thanks for posting this! I've put up a fix in #29591. Our apologies for overlooking this issue earlier.

Note that it's okay to modify a global variable during an event handler or an effect, but be careful that the child component doesn't call this function during render.

josephsavona avatar May 25 '24 21:05 josephsavona

@josephsavona thanks for fix, but you are merged this fix into babel-plugin-react-compiler package, but ESLint will throw error on it

adubrouski avatar May 29 '24 15:05 adubrouski

@adubrouski the eslint plugin is powered by the babel plugin, this fixes both.

josephsavona avatar May 29 '24 17:05 josephsavona

@josephsavona amazing work! Are you planning to publish a new version of the ESLint plugin soon?

NMinhNguyen avatar May 29 '24 17:05 NMinhNguyen

Is there a hint to let the arg know that's a ref? Or is there another pattern I should be using? I'm still getting this error, but maybe I'm doing something incorrectly.

function useAdjustScroll(ref: React.RefObject<HTMLDivElement>) {
  React.useEffect(() => {
    if (ref.current) {
      ref.current.scrollTop = 0;
    }
  }, [ref]);
}

https://playground.react.dev/#N4Igzg9grgTgxgUxALhAMygOzgFwJYSYAEUYCAggCYBWpOAynDBADYsAUMCayRASggCGuAHQC0AeQBG1BLgA8ACQAqAWQAyAETwA3AKIsEAWwSYcAPgCURYAB1i-IaNII9aNHJzt21gLzmbeyJgojw0Ik5uEThYLjNrOwcQoi40aNjTHBEwJlYWZQgAByJfIgAGAG4gkIBfapqAGiIAbVSAXUsqzBqQGqA

enableTreatRefLikeIdentifiersAsRefs that seems to solve this problem, but i have no idea how this works with just the babel plugin

rostero1 avatar Jul 25 '24 19:07 rostero1