-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[code-infra] Stabilize size for bundles with releaseInfo
#19674
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
Conversation
Deploy preview: https://deploy-preview-19674--material-ui-x.netlify.app/ Bundle size report
|
increase caused by vite preload code being included in the output |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
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.
Looks perfect!
releaseInfo
upload: !!process.env.CI, | ||
replace: { | ||
// Stabilize release info string | ||
[JSON.stringify(generateReleaseInfo())]: JSON.stringify('__RELEASE_INFO__'), |
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.
@Janpot Looking at the usage, shouldn't the key and values should be other way round, ie,
replace: {
// Stabilize release info string
[JSON.stringify('__RELEASE_INFO__')]: JSON.stringify(generateReleaseInfo()),
}
which would mean that look for __RELEASE_INFO__
and replace with the generateReleaseInfo()
value.
Also, the JSON.stringify()
call can probably be part of the bundle-size-checker package to simplify the config in repos.
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.
Looking at the usage, shouldn't the key and values should be other way round
No, this PR attempts to exactly revert what X does in their bundles. So we want releaseInfo
to be set back to "__RELEASE_INFO__"
. That way this value will be constant across releases and not cause slight differences in gzip.
Also, the
JSON.stringify()
call can probably be part of the bundle-size-checker package to simplify the config in repos.
JSON.stringify()
is there to add the quotes with automatic escaping. I think we want to keep supporting the case where you want to replace a value that is not a string literal.
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.
Got it. So it still supports the other search: value
use-case. This config is just specific to mui-x
.
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.
Yes, it's similar to how define
works. Just for more than only top-level constants.
Close mui/mui-public#721