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.

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

[core] Fix some issues reported by eslint-plugin-react-compiler #43117

Merged
merged 26 commits into from
Sep 12, 2024
Merged

[core] Fix some issues reported by eslint-plugin-react-compiler #43117

merged 26 commits into from
Sep 12, 2024

Conversation

binsmyth
Copy link
Contributor

@binsmyth binsmyth commented Jul 30, 2024 •
edited by aarongarciah
Loading

Part of #42564

Sorry for being late to contribute.
I changed the codes in these files to remove errors -

  • packages/mui-material/src/Backdrop/Backdrop.js
  • packages/mui-material/src/usePagination/usePagination.test.js
  • packages/mui-material/src/FormControl/FormControl.js
    While rest of them I put eslint disable comments. Either difficult to change or could break something.

For this package there was two changes didn't realize that the change caused test to not work so undid the changes and added disabled comments

@mui-bot
Copy link

mui-bot commented Jul 30, 2024 •
edited
Loading

Netlify deploy preview

https://deploy-preview-43117--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against cd0f9fa

@zannager zannager added core Infrastructure work going on behind the scenes package: eslint Specific to eslint-plugin-material-ui labels Jul 30, 2024
@aarongarciah aarongarciah removed the package: eslint Specific to eslint-plugin-material-ui label Aug 5, 2024
@aarongarciah aarongarciah changed the title Fix eslint issues component hooks [core] Fix eslint issues component hooks Aug 5, 2024
@aarongarciah aarongarciah changed the title [core] Fix eslint issues component hooks [core] Fix issues reported by eslint-plugin-react-compiler Aug 5, 2024
aarongarciah
aarongarciah reviewed Aug 5, 2024
View reviewed changes
Copy link
Member

@aarongarciah aarongarciah left a comment

Choose a reason for hiding this comment

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

Thanks for working on this. I left some comments.

packages/mui-material/src/Hidden/withWidth.js Outdated Show resolved Hide resolved
packages/mui-material/src/InputBase/InputBase.js Outdated Show resolved Hide resolved
packages/mui-material/src/InputBase/InputBase.js Outdated Show resolved Hide resolved
packages/mui-material/src/InputBase/InputBase.test.js Outdated Show resolved Hide resolved
packages/mui-material/src/InputBase/InputBase.test.js Outdated Show resolved Hide resolved
packages/mui-material/src/Select/SelectInput.js Outdated Show resolved Hide resolved
packages/mui-material/src/SwipeableDrawer/SwipeableDrawer.js Outdated Show resolved Hide resolved
packages/mui-material/src/Tooltip/Tooltip.js Outdated Show resolved Hide resolved
packages/mui-material/src/Tooltip/Tooltip.js Outdated Show resolved Hide resolved
packages/mui-material/src/Tooltip/Tooltip.js Outdated Show resolved Hide resolved
@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Aug 6, 2024
@aarongarciah
Copy link
Member

@binsmyth there are comments that haven't been addressed (e.g. https://github.com/mui/material-ui/pull/43117/files#r1703678961). Please, go through all of them and reply accordingly before requesting a new review.

@binsmyth
Copy link
Contributor Author

@aarongarciah Sorry must have missed it.I will check it. thanks

@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Sep 9, 2024
binsmyth and others added 14 commits September 12, 2024 15:36
@binsmyth @aarongarciah
prevent value mutation
cfe6f39
@binsmyth @aarongarciah
Put conditionals after useRef
6602e24
@binsmyth @aarongarciah
disable compiler because can't prevent reassignment
a9171bf
@binsmyth @aarongarciah
put variable change inside useEffect
6262ef5
@binsmyth @aarongarciah
add other as dependency to useEffect
8eafa61
@binsmyth @aarongarciah
amend previous changes because test not passing
04ee7d4
@binsmyth @aarongarciah
add lines to disable compiler
a74e61a
@binsmyth @aarongarciah
add line to disable react compiler for lines
6b8b6f0
@binsmyth @aarongarciah
add line to disable compiler
78230f5
@binsmyth @aarongarciah
fixed prettier errors
6c90373
@binsmyth @aarongarciah
Update packages/mui-material/src/Hidden/withWidth.js
f8338aa 
Co-authored-by: Aarón García Hervás <[email protected]>
Signed-off-by: binsmyth <[email protected]>
@binsmyth @aarongarciah
Update packages/mui-material/src/InputBase/InputBase.js
a78f175 
Co-authored-by: Aarón García Hervás <[email protected]>
Signed-off-by: binsmyth <[email protected]>
@binsmyth @aarongarciah
Update packages/mui-material/src/InputBase/InputBase.test.js
4850d61 
Co-authored-by: Aarón García Hervás <[email protected]>
Signed-off-by: binsmyth <[email protected]>
@binsmyth @aarongarciah
Update packages/mui-material/src/InputBase/InputBase.js
4aa3c76 
Co-authored-by: Aarón García Hervás <[email protected]>
Signed-off-by: binsmyth <[email protected]>
binsmyth and others added 12 commits September 12, 2024 15:43
@binsmyth @aarongarciah
Update packages/mui-material/src/InputBase/InputBase.test.js
b4ea6ff 
Co-authored-by: Aarón García Hervás <[email protected]>
Signed-off-by: binsmyth <[email protected]>
@binsmyth @aarongarciah
Update packages/mui-material/src/Select/SelectInput.js
ccbc732 
Co-authored-by: Aarón García Hervás <[email protected]>
Signed-off-by: binsmyth <[email protected]>
@binsmyth @aarongarciah
Update packages/mui-material/src/SwipeableDrawer/SwipeableDrawer.js
43b9fd2 
Co-authored-by: Aarón García Hervás <[email protected]>
Signed-off-by: binsmyth <[email protected]>
@binsmyth @aarongarciah
Update packages/mui-material/src/Tooltip/Tooltip.js
c885501 
Co-authored-by: Aarón García Hervás <[email protected]>
Signed-off-by: binsmyth <[email protected]>
@binsmyth @aarongarciah
Update packages/mui-material/src/Tooltip/Tooltip.js
ba82d4a 
Co-authored-by: Aarón García Hervás <[email protected]>
Signed-off-by: binsmyth <[email protected]>
@binsmyth @aarongarciah
Update packages/mui-material/src/Tooltip/Tooltip.js
c82e138 
Co-authored-by: Aarón García Hervás <[email protected]>
Signed-off-by: binsmyth <[email protected]>
@binsmyth @aarongarciah
Update packages/mui-material/src/useScrollTrigger/useScrollTrigger.js
b66cc4a 
Co-authored-by: Aarón García Hervás <[email protected]>
Signed-off-by: binsmyth <[email protected]>
@binsmyth @aarongarciah
Update packages/mui-material/src/styles/useTheme.js
83c46d9 
Co-authored-by: Aarón García Hervás <[email protected]>
Signed-off-by: binsmyth <[email protected]>
@binsmyth @aarongarciah
Update packages/mui-material/src/Snackbar/Snackbar.test.js
fa715b9 
Co-authored-by: Aarón García Hervás <[email protected]>
Signed-off-by: binsmyth <[email protected]>
@binsmyth @aarongarciah
Update packages/mui-material/src/Popover/Popover.js
082da68 
Co-authored-by: Aarón García Hervás <[email protected]>
Signed-off-by: binsmyth <[email protected]>
@binsmyth @aarongarciah
Update packages/mui-material/src/Select/SelectInput.js
6b11060 
Co-authored-by: Aarón García Hervás <[email protected]>
Signed-off-by: binsmyth <[email protected]>
@binsmyth @aarongarciah
Revert "put variable change inside useEffect"
cd0f9fa 
This reverts commit 132eda3.
@aarongarciah aarongarciah force-pushed the fix-eslint-issues-component-hooks branch from 886680d to cd0f9fa Compare September 12, 2024 13:45
@aarongarciah aarongarciah changed the title [core] Fix issues reported by eslint-plugin-react-compiler [core] Fix some issues reported by eslint-plugin-react-compiler Sep 12, 2024
aarongarciah
aarongarciah approved these changes Sep 12, 2024
View reviewed changes
Copy link
Member

@aarongarciah aarongarciah left a comment

Choose a reason for hiding this comment

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

Thanks!

@aarongarciah aarongarciah merged commit 47cc78d into mui:master Sep 12, 2024
19 checks passed
@aarongarciah aarongarciah mentioned this pull request Sep 12, 2024
Open
66 tasks
@wilhelmlofsten wilhelmlofsten mentioned this pull request Sep 30, 2024
Merged
1 task
@duy0000 duy0000 mentioned this pull request Oct 25, 2024
Open
This was referenced Oct 25, 2024
Open
Open
@bansikah22 bansikah22 mentioned this pull request Oct 26, 2024
Merged
@Eskedar21 Eskedar21 mentioned this pull request Oct 26, 2024
Open
@josetutis josetutis mentioned this pull request Oct 26, 2024
Open
@Hoang-Nguyen-Huy Hoang-Nguyen-Huy mentioned this pull request Oct 26, 2024
Merged
This was referenced Oct 29, 2024
Open
Open
@wilhelmlofsten wilhelmlofsten mentioned this pull request Nov 8, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aarongarciah aarongarciah aarongarciah approved these changes

Assignees
No one assigned
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@binsmyth @mui-bot @aarongarciah @zannager