-
-
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
[core] Use MUI X official name in errors #11645
[core] Use MUI X official name in errors #11645
Conversation
Deploy preview: https://deploy-preview-11645--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.
LGTM. 👌
One note: we decided to use MUI-X-Charts
as the error prefix on x-charts
code. 🤔
WDYT about that?
Would you rather see MUI X
for every package or make the error prefixes package-specific?
I.e.:
MUI-X-DataGrid
MUI-X-Pickers
MUI-X-Charts
P.S. I took the liberty to change the label and PR prefix as it seems to target the core
code rather than docs
. 🤔
The Otherwise, I think that a broad "MUI X" prefix could be enough, most of the time the error comes with the React component stack trace, so it's not too hard to figure out where the problem is. |
Adding the |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
92dea9e
to
a04e743
Compare
Yes, My PR was to be sure all charts error have the same prefix. I don't care about which one. I added charts because errors of this package can be tricky, so if I can make sure users understand which component is complaining, it's good to take |
Alright, then a few small steps:
In http://github.com/mui/material-ui, I think that it will be good to make the same move, to replace MUI and instead clarify what is MUI System, Material UI or Base UI errors cc @danilo-leal @samuelsycamore. This would be a continuation of #11556 (comment). |
179444d
to
33b3fd9
Compare
33b3fd9
to
348be72
Compare
Issue created for the core projects: mui/material-ui#40590 to not forget about this. |
Assuming #11556 (comment) logic holds, then this change seems to make the most sense.