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

[pickers] Add @mui/base to peerDependencies #8590

Merged
merged 2 commits into from
Apr 13, 2023

Conversation

LukasTy
Copy link
Member

@LukasTy LukasTy commented Apr 12, 2023

Avoids an issue discovered #8530 (comment)

Also synced all peerDependencies between community and pro packages.

@LukasTy LukasTy added core Infrastructure work going on behind the scenes component: pickers This is the name of the generic UI component, not the React module! labels Apr 12, 2023
@LukasTy LukasTy self-assigned this Apr 12, 2023
@mui-bot
Copy link

mui-bot commented Apr 12, 2023

Netlify deploy preview

Netlify deploy preview: https://deploy-preview-8590--material-ui-x.netlify.app/

Updated pages

No updates.

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 728.6 1,222 1,222 987.38 211.375
Sort 100k rows ms 868.3 1,340.5 1,202.4 1,143.58 153.83
Select 100k rows ms 248.8 355.7 327 306.92 42.201
Deselect 100k rows ms 252.1 398 306.3 306.74 51.645

Generated by 🚫 dangerJS against 7e22104

@LukasTy LukasTy merged commit 10162e2 into mui:master Apr 13, 2023
@LukasTy LukasTy deleted the update-peer-deps branch April 13, 2023 07:30
@oliviertassinari
Copy link
Member

Why not a direct dependency? From what I understand, there are no singleton in Base UI that would break the app if it's duplicated.

@LukasTy
Copy link
Member Author

LukasTy commented Apr 17, 2023

Why not a direct dependency? From what I understand, there are no singleton in Base UI that would break the app if it's duplicated.

My thinking was that we currently support @mui/x-* package usage only with @mui/material anyways and that package already brings its own version of @mui/base also, we have the @mui/system dependency listed in the same way. 🤔
But now, when you asked it again...
Couldn't we just add both the @mui/base as well as @mui/system packages as dependencies?
At least the @mui/x-date-pickers package can't function without these packages and @mui/material already has them as dependencies. 🤔

@LukasTy LukasTy mentioned this pull request May 19, 2023
45 tasks
@oliviertassinari
Copy link
Member

oliviertassinari commented May 21, 2023

Couldn't we just add both the @mui/base as well as @mui/system packages as dependencies?

👍 for @mui/base. I see it listed in #7902 but it's not a breaking change, as far as I know. This way, we can also broaden the range (reduced in #8531).

For @mui/system, to consider that it forces dependencies for the folks that want to use the headless version of the date picker. I believe @mui/system is in the same boat as @mui/material as to why an optional peer dependency can make more sense (only if you use the Material UI view).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pickers This is the name of the generic UI component, not the React module! core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants