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

[styles] Fix issues reported by eslint-plugin-react-compiler #43118

Merged
merged 4 commits into from
Aug 14, 2024

Conversation

jlewins
Copy link
Contributor

@jlewins jlewins commented Jul 30, 2024

Part of #42564

FILES CHANGED:

StylesProvider.js: ESLint error: "Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect." Line 87. Moved variable definition of 'injectFirstNode' inside of 'StylesProvider' function, fixing ESLint error

makeStyles.js: ESLint error: "Expected the dependency list for useMemo to be an array literal" Line 167. Changed 'useMemo' param "values" to "[values]", fixing ESLint error

makeStyles.spec.tsx: ESLint error: "Hooks must be the same function on every render but this value may change over time to a different function." Line 132. Moved definition of 'useStyles' outside of function 'PartialTypeInferenceTest' to align with rules of hooks, fixing ESlint error

withStyles.js: ESLint error: "React Compiler has skipped optimizing this component because one or more React ESLint rules were disabled. React Compiler only works when your components follow all the rules of React, disabling them may result in unexpected or incorrect behavior" Line 60. Unsure of common practice for this error, and was unable to find other examples in repo so fixed via silencing for now but advice would be appreciated.

@jlewins

This comment was marked as duplicate.

@mui-bot
Copy link

mui-bot commented Jul 30, 2024

Netlify deploy preview

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

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 4210f19

@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
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.

Left some comments

packages/mui-styles/src/StylesProvider/StylesProvider.js Outdated Show resolved Hide resolved
packages/mui-styles/src/makeStyles/makeStyles.js Outdated Show resolved Hide resolved
packages/mui-styles/src/makeStyles/makeStyles.js Outdated Show resolved Hide resolved
packages/mui-styles/src/withStyles/withStyles.js Outdated Show resolved Hide resolved
@aarongarciah aarongarciah changed the title Fixed ESLint Errors in mui-styles [styles] Fix issues reported by eslint-plugin-react-compiler Aug 12, 2024
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 @jlewins!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants