-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[charts] Fix eslint for react compiler #13444
Conversation
Deploy preview: https://deploy-preview-13444--material-ui-x.netlify.app/ |
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.
Smart move changing the warnedOnce
into the closure 👍
@@ -0,0 +1,17 @@ | |||
export function buildWarning( |
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.
I would probably just rename it to buildOneTimeWarning
or buildWarningOnce
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.
It's a common piece of code we duplicate in each package.
Was introduced mostly to avoid code duplication in the data grid. We could do the renaming but in that case, would require to do it in all packages
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.
up to you, doesn't matter much 😅
should we eventually have a shared X package? shouldn't be too hard to accomplish
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.
should we eventually have a shared X package?
This has been discussed quite recently
I feel like we are only waiting for a use-case and someone to own the creation of the new package 😆
@joserodolfofreitas @cherniavskii if you are OK, we can create it
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.
Agreed, we need a shared utils package 👍🏻
Not sure it's the best way to fix warnings about no side effect, but it works 😇