-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[charts][infra] Enable import/no-cycle and @typescript-eslint/consistent-type-imports eslint rules
#20554
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
base: master
Are you sure you want to change the base?
Conversation
Localization writing tips ✍️Seems you are updating localization 🌍 files. Thank you for contributing to the localization! 🎉 To make your PR perfect, here is a list of elements to check: ✔️
Deploy preview: https://deploy-preview-20554--material-ui-x.netlify.app/ Bundle size report
|
CodSpeed Performance ReportMerging #20554 will not alter performanceComparing Summary
|
| return reverse ? !invertStartCoordinate : invertStartCoordinate; | ||
| } | ||
|
|
||
| export function getBarDimensions(params: { |
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 main cyclic change here was that I had to move this function from packages/x-charts/src/BarChart/useBarPlotData.ts into its own file
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.
Can we isolate the changes in a different PR? Ideally in this PR we'd only enable the ESLint rules and adds the type imports. Otherwise it's impossible to review ~500 files
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.
You will still have to trust me I didn't change any file though 😆
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 not a trust question, it's to split code modification from type modification ;)
I 100% trust you to introduce bugs :p
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 100% trust you to introduce bugs :p
Glad I'm hold to a high standard. It makes me thank my long history of spectacular bug implementations.
I was finding cryptic errors when running #20404 in development mode.
It took me a long time to figure out the issue was cyclic deps and fix them, so we might as well enable the plugin instead 😆
The
@typescript-eslint/consistent-type-importsis important becausetype importsare not counted towards cyclic deps, as they are stripped when compiling. This makes it much easier to find the correct cyclic issues, eg: it was something like 400 errors originally and about 27 after.