Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
NickBlow opened this issue May 16, 2024 · 8 comments

Comments

@NickBlow
Copy link

NickBlow commented May 16, 2024

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?

@HoseinKhanBeigi
Copy link

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) => {
event.preventDefault();

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

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

};`

@codingedgar
Copy link

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
Copy link

codingedgar commented May 16, 2024

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.

@wojtekmaj
Copy link

wojtekmaj commented May 17, 2024

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

@josephsavona
Copy link
Contributor

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.

@adubrouski
Copy link

adubrouski commented May 29, 2024

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

@josephsavona
Copy link
Contributor

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

@NMinhNguyen
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants