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 & eslint plugin giving an error to mutating values in refs that are used in JSX #29703

Closed
adubrouski opened this issue Jun 1, 2024 · 7 comments

Comments

@adubrouski
Copy link

adubrouski commented Jun 1, 2024

Summary

eslint-plugin-react-compiler version: 0.0.0-experimental-51a85ea-20240601

When I try to mutate a ref value in any event handler, the eslint-plugin-react compiler gives 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/#N4Igzg9grgTgxgUxALhAgHgBwjALgAgBMEAzAQygBsCSoA7OXASwjvwEFNMAKASn2AAdNvjiswNJpQQBJOpii4ASqXwBefFDAIVJADwAJACoBZADJyFuAKLSAtgjq4AfNzpVKvANzDh+UeIEABZkdITSMnbYeOr4fOrOAn7++EwkcSRSsvKKugB0cLAwjgQAhGoa7pSeSSIp+JkROcqkBUUleQBuZJRQCLGCIIM+dSmN2Vb5hTDFTgWUTHAA1nwj9QC+yesjycW4sGzcyf56zscpekzN+LgAnpgIakIg44Pr+MUkT+OWuaTvAHoznRzicAEaKXCsfCsADCC2WTxCYQiURwuHWzki0VwegBENwULowPqeJJ+G8wk2dBA6yAA

This seems a bit off to me because the documentation specifically mentions manipulating the DOM with a ref as an accepted pattern:
https://react.dev/reference/react/useRef#manipulating-the-dom-with-a-ref

@josephsavona made a fix for issue like this, but this fix allow only global mutations like window or document. Refs mutations also cause an error in eslint-plugin-react-compiler

@adubrouski adubrouski changed the title [React 19] react-compiler-eslint-plugin giving an error to mutating values in refs that are used in JSX [React 19] React compiler & eslint plugin giving an error to mutating values in refs that are used in JSX Jun 1, 2024
@josephsavona
Copy link
Contributor

josephsavona commented Jun 1, 2024

Good catch, this is a bug. We’ll need to add a new FunctionEffect variant for writes to refs, and allow that effect inside JSX and effects.

@coryhouse
Copy link
Contributor

Thanks @josephsavona. A related issue: If a ref is passed as a prop, the compiler can't tell it's a prop, so the linter warns if the prop is mutated. If you fix the bug above, will the linter allow mutating refs passed as props?

@gsathya
Copy link
Member

gsathya commented Jun 3, 2024

Thanks @josephsavona. A related issue: If a ref is passed as a prop, the compiler can't tell it's a prop, so the linter warns if the prop is mutated. If you fix the bug above, will the linter allow mutating refs passed as props?

This one is slightly different, I created another issue to track that separately here: #29741

@gsathya gsathya self-assigned this Jun 3, 2024
josephsavona added a commit that referenced this issue Jun 7, 2024
Fixes false positives where we currently disallow mutations of refs from callbacks passed to JSX, if the ref is also passed to jsx. We consider these to be mutations of "frozen" values, but refs are explicitly allowed to have interior mutability.

The fix is to add a new RefMutation variant to FunctionEffect, which we emit whenever a ref is mutated. We disallow this effect type from within a function body, but allow it within function expressions that are passed to useEffect and friends as well as to jsx.

Fixes #29741
Fixes #29703

[ghstack-poisoned]
josephsavona added a commit that referenced this issue Jun 7, 2024
Fixes false positives where we currently disallow mutations of refs from callbacks passed to JSX, if the ref is also passed to jsx. We consider these to be mutations of "frozen" values, but refs are explicitly allowed to have interior mutability.

The fix is to add a new RefMutation variant to FunctionEffect, which we emit whenever a ref is mutated. We disallow this effect type from within a function body, but allow it within function expressions that are passed to useEffect and friends as well as to jsx.

Fixes #29741
Fixes #29703

[ghstack-poisoned]
@josephsavona
Copy link
Contributor

Fixed in #29733

@adubrouski
Copy link
Author

@josephsavona thanks for fix. Are there any plans to publish a new version of the compiler and ESLint plugin soon?

@josephsavona
Copy link
Contributor

Yeah we’ll publish again next week

@mziemer21
Copy link

@josephsavona any updates on publishing?

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

5 participants