-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
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
[docs-infra] Fix React Compiler ESLint issues in website components #42566
[docs-infra] Fix React Compiler ESLint issues in website components #42566
Conversation
Netlify deploy previewhttps://deploy-preview-42566--material-ui.netlify.app/ Bundle size report |
@@ -101,7 +101,10 @@ export const useResizeHandle = ( | |||
if (target.current && dragging && clientX) { | |||
const objectRect = target.current.getBoundingClientRect(); | |||
const newWidth = clientX - objectRect.left + dragOffset; | |||
target.current.style.width = `clamp(${minWidth}, ${Math.floor(newWidth)}px, ${maxWidth})`; | |||
target.current.style.setProperty( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using style.setProperty
so there's no "mutation".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels strange: facebook/react#29832.
Looking into the reported ESLint errors:
We use the |
I've pushed a change commenting the ESLint disable comments for the - // eslint-disable-next-line react-compiler/react-compiler -- useEnhancedEffect uses useEffect under the hood
+ // TODO: uncomment once we enable eslint-plugin-react-compiler // eslint-disable-next-line react-compiler/react-compiler -- useEnhancedEffect uses useEffect under the hood We can't have disabled rules that are not configured because we use the --report-unused-disable-directives ESLint flag. |
@@ -101,7 +101,10 @@ export const useResizeHandle = ( | |||
if (target.current && dragging && clientX) { | |||
const objectRect = target.current.getBoundingClientRect(); | |||
const newWidth = clientX - objectRect.left + dragOffset; | |||
target.current.style.width = `clamp(${minWidth}, ${Math.floor(newWidth)}px, ${maxWidth})`; | |||
target.current.style.setProperty( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels strange: facebook/react#29832.
7d56792
to
e861ac6
Compare
@@ -475,7 +475,7 @@ module.exports = { | |||
rules: { | |||
'import/no-default-export': 'error', | |||
'import/prefer-default-export': 'off', | |||
...(ENABLE_REACT_COMPILER_PLUGIN ? { 'react-compiler/react-compiler': 'off' } : {}), | |||
'react-compiler/react-compiler': 'off', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is ENABLE_REACT_COMPILER_PLUGIN
no longer useful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now we leave it always off for Base UI.
@@ -166,6 +166,7 @@ function PersistScroll(props) { | |||
} | |||
|
|||
return () => { | |||
// TODO: uncomment once we enable eslint-plugin-react-compiler // eslint-disable-next-line react-compiler/react-compiler -- useEnhancedEffect uses useEffect under the hood |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the plan with these comments? To fix it in the future or to keep the disable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The plan is to keep the disable but remove the comment/TODO once we enable the corresponding ESLint plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Part of #42564
Fixes or silences errors reported by
eslint-plugin-react-compiler
in the website components.The silenced occurrences are prepended with a comment because we use the --report-unused-disable-directives and we can't silence rules that are not configured, like the ones coming from
eslint-plugin-react-compiler
, which is disabled by default as of today.// TODO: uncomment once we enable eslint-plugin-react-compiler // eslint-disable-next-line react-compiler/react-compiler -- useEnhancedEffect uses useEffect under the hood
Apart from that, add a missing React key.
How to test
Make sure the parts of the docs that are affected by the changes work as expected. Half of the changes are just silencing the corresponding ESLint rule.