Skip to content

[compiler] Silence ref-in-render errors in ESLint #30843

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: gh/josephsavona/52/base
Choose a base branch
from
Open

[compiler] Silence ref-in-render errors in ESLint #30843

wants to merge 3 commits into from

Conversation

josephsavona
Copy link
Member

@josephsavona josephsavona commented Aug 28, 2024 •
edited
Loading

Stack from ghstack (oldest at bottom):

We need this for compiler correctness but the false positive rate is a bit too high, let's suppress in ESLint for now.

@josephsavona
[compiler] Silence ref-in-render errors in ESLint
5c88fac 
We need this for compiler correctness but the false positive rate is a bit too high, let's suppress in ESLint for now.

[ghstack-poisoned]
@vercel Vercel
Copy link

vercel bot commented Aug 28, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 30, 2024 5:46am

josephsavona added a commit that referenced this pull request Aug 28, 2024
@josephsavona
[compiler] Silence ref-in-render errors in ESLint
1c0ab0b 
We need this for compiler correctness but the false positive rate is a bit too high, let's suppress in ESLint for now.

ghstack-source-id: 6dc16cf
Pull Request resolved: #30843
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Aug 28, 2024
poteto
poteto reviewed Aug 28, 2024
View reviewed changes
compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts Outdated
* Code that breaks the rules of React, but which should not be reported in ESLint due to
* false positives.
*/
SilentInvalidReact = 'SilentInvalidReact',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm wait, doesn't this PR need to update the ValidateRefAccess pass with the new severity too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah i had a bad revert, look now!

@josephsavona
Update on "[compiler] Silence ref-in-render errors in ESLint"
dfa80c4 
We need this for compiler correctness but the false positive rate is a bit too high, let's suppress in ESLint for now.

[ghstack-poisoned]
josephsavona added a commit that referenced this pull request Aug 28, 2024
@josephsavona
[compiler] Silence ref-in-render errors in ESLint
c741d2d 
We need this for compiler correctness but the false positive rate is a bit too high, let's suppress in ESLint for now.

ghstack-source-id: 2d24397
Pull Request resolved: #30843
@josephsavona josephsavona mentioned this pull request Aug 29, 2024
Merged
@josephsavona
Update on "[compiler] Silence ref-in-render errors in ESLint"
57f60bd 
We need this for compiler correctness but the false positive rate is a bit too high, let's suppress in ESLint for now.

[ghstack-poisoned]
josephsavona added a commit that referenced this pull request Aug 30, 2024
@josephsavona
[compiler] Silence ref-in-render errors in ESLint
7f6c049 
We need this for compiler correctness but the false positive rate is a bit too high, let's suppress in ESLint for now.

ghstack-source-id: 8218c3a
Pull Request resolved: #30843
@lukpsaxo
Copy link

If you decide to merge this, please can you make it an option? Otherwise for people who already upgraded and added hundreds of eslint suppression comments, would have to undo/remove those and then presumably re-add them at some later point in time.

@andrey-medvedev-vk andrey-medvedev-vk mentioned this pull request Sep 23, 2024
Closed
@tyxla tyxla mentioned this pull request Oct 7, 2024
Merged
@ntdiary ntdiary mentioned this pull request Oct 23, 2024
Merged
51 tasks
@roryabraham roryabraham mentioned this pull request Jan 28, 2025
Closed
8 tasks
@jroitgrund
Copy link

We need this for compiler correctness but the false positive rate is a bit too high, let's suppress in ESLint for now.

Does this mean that despite this ESLint rule change, the compiler will continue to skip optimizing components which render ref.current? This is being discussed here: pmndrs/zustand#2995

@jroitgrund jroitgrund mentioned this pull request Feb 4, 2025
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@poteto poteto poteto left review comments

@mofeiZ mofeiZ mofeiZ approved these changes

Assignees
No one assigned
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@josephsavona @lukpsaxo @jroitgrund @poteto @mofeiZ @facebook-github-bot