-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Bump MUI infra packages #20128
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?
Bump MUI infra packages #20128
Conversation
6e269e1 to
9b85bb6
Compare
|
Deploy preview: https://deploy-preview-20128--material-ui-x.netlify.app/ Bundle size report
|
9b85bb6 to
c138cbc
Compare
c138cbc to
4e93100
Compare
4e93100 to
fe8d474
Compare
fe8d474 to
662c128
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
662c128 to
a0e52b9
Compare
eslint.config.mjs
Outdated
| ], | ||
| }, | ||
| })), | ||
| // FIXME: Remove these exceptions once the migration is complete |
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.
Disabled the rules because lint was failing with around 1000 problems.
I'll let teams know that they should try to remove their packages from here as they fix the issues.
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 reference: https://github.com/mui/mui-x/pull/20120/files#r2481552661
The new version of react-hooks plugin contains the compiler rules. Which are riddled with false positives. I personally wouldn't necessarily spend a lot of time on it yet until we decided the path forward with react compiler.
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.
Thanks for the context 👍
Edited/Blocked NotificationRenovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR. You can manually request rebase by checking the rebase/retry box above. |
eslint.config.mjs
Outdated
| files: [`packages/${pkgName}/src/**/*${EXTENSION_TS}`], | ||
| ignores: ['**/*.d.ts'], | ||
| rules: { | ||
| 'import/export': '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.
Did we have import/export rule errors in these packages?
Disabling it globally seems like an overkill
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.
There's some weird bug I've observed when we're re-exporting from a barrel file that itself re-exports from a barrel file in another workspace.
Until we can pinpoint this, we should probably just eslint-disable those specific lines.
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'm only disabling it in the packages that have lint errors, which are the ones defined in line 446.
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.
Until we can pinpoint this, we should probably just eslint-disable those specific lines.
The charts team has an export-generation script to ensure consistent exports across community, pro and premium packages that uses the offending syntax.
We could disable it line by line, but I'd need to update the script.
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.
If I understand correctly, exporting the same name multiple times can cause issues in ES modules, and the rule itself seems to make sense. So I'm just trying to be cautious here and avoid potential issues introduced after the rule is disabled.
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.
Disabled it line by line
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.
If I understand correctly, exporting the same name multiple times can cause issues in ES modules
The problem I observed didn't have to do with re-exports, and rather seems to be a eslint-plugin-import bug.
The charts team has an export-generation script to ensure consistent exports across community, pro and premium packages that uses the offending syntax.
A good demonstration of why it's often better to verify in a lint step rather than generate sources. It leaves the sources available to hand edit.
47abe25 to
8b95c57
Compare
This PR contains the following updates:
^1.0.4-canary.7->^1.0.4-canary.8^1.0.9-canary.53->^1.0.9-canary.54^0.0.3-canary.38->^0.0.3-canary.44Configuration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.
This PR was generated by Mend Renovate. View the repository job log.