[Popper] Fix bundle size regression #27910
Conversation
oliviertassinari
added
component: Popper
|
As far as I know, the regression comes from #25984. But it wasn't reported, nor does the improvement here is reported in the mui-pr-bot's comment. I had to open the links to check. I would propose we display a summary of the important bundle size changes in the mui-pr-bot's comment. cc @eps1lon |
| @@ -2,7 +2,7 @@ import * as React from 'react'; | |||
| import PropTypes from 'prop-types'; | |||
| import { createPopper } from '@popperjs/core'; | |||
| import { chainPropTypes, refType, HTMLElementType } from '@material-ui/utils'; | |||
| import { useTheme } from '../styles'; | |||
| import { useThemeWithoutDefault as useTheme } from '@material-ui/system'; | |||
There was a problem hiding this comment.
This is part of the core library so it needs the default theme. Same applies to the other components.
There was a problem hiding this comment.
Please explain why the default theme matters in this context. I don't see the link with the core.
To be clear, we haven't been using the default theme for a long time here. I have made this PR quickly after we changed the behavior.
There was a problem hiding this comment.
Please explain why the default theme matters in this context. I don't see the link with the core.
To be clear, we haven't been using the default theme for a long time here. I have made this PR quickly after we changed the behavior.
I forgot that you just argue for consistency when it suites you. But generally, the Popper is not conform with the other /material components regarding default props.
There was a problem hiding this comment.
By not conform, do you mean that there is a difference in behavior? I didn't notice it, if you could highlight it, I think that it would be great. As far as I know, there are no behavior changes. This PR trims default values of the theme that the component doesn't depend on.
Regarding consistency, I personally don't think that it's a target on its own, but a tool to 1. surface miss opportunities in how we solve problems, e.g. Does it really need two different solutions here? 2. to minimize the learning curve e.g. As a user, to be able to transfer API knowledge from one component to another.
Regarding what suites me, I think that it's about: Are we OK to cherry-pick only the default values of the theme the component needs for the sake of reducing the bundle size when importing a single component? I personally think that the answer is yes because it ease the adoption of the library. As a developer you do no longer feel that you have to import multiple components to get a return on the initial bundle size overhead.
github-actions
bot
added
the
PR: out-of-date
oliviertassinari
force-pushed
the
fix-popper-bundle-size-regression
branch
from
46bf44d
to
77b7a77
Compare
github-actions
bot
removed
the
PR: out-of-date
I have noticed this I was working on the v5 blog post.
https://mui-dashboard.netlify.app/size-comparison?buildId=32104&baseRef=next&baseCommit=83cb85545ed1838a93e0ff69394885a3b7dd1830&prNumber=27910