-
-
Notifications
You must be signed in to change notification settings - Fork 32.7k
[code-infra] Migrate build command to code-infra #46614
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
Netlify deploy previewhttps://deploy-preview-46614--material-ui.netlify.app/ Bundle size report
|
122ca48
to
6435e26
Compare
0e5759f
to
dd11aaa
Compare
dd11aaa
to
94bc1f3
Compare
c7e7703
to
18e6a1b
Compare
18e6a1b
to
e0c9988
Compare
e0c9988
to
44169ac
Compare
44169ac
to
7c32acb
Compare
65484d7
to
953ff45
Compare
953ff45
to
ed0cdfa
Compare
3419612
to
11c0241
Compare
Any idea about the bundle size differences? |
@@ -1,24 +1,29 @@ | |||
// @ts-check |
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.
Why remove?
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.
Added back. Forgot to copy this into the new file.
@@ -3,7 +3,6 @@ | |||
"version": "2.0.12", | |||
"author": "MUI Team", | |||
"description": "Utilities supporting MUI libraries build and docs generation. This is an internal package not meant for general use.", | |||
"main": "build/index.js", |
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.
Doesn't have to part of this scope, but maybe we could do some "package.json linting" in our build step? To error or warn on things like this?
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 do linting for exports
field right now (the one without *
). main
field is anyways overwritten based on the presence of build/index.js
.
But I agree, we can always add more checks, even for the ones with *
.
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, I mean if it's overwritten, we may as well just warn at least
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.
Noted. Will add in code-infra
.
scripts/build.mjs
Outdated
@@ -55,7 +55,7 @@ async function run(argv) { | |||
); | |||
} | |||
|
|||
const babelConfigPath = path.resolve(getWorkspaceRoot(), 'babel.config.js'); | |||
const babelConfigPath = path.resolve(getWorkspaceRoot(), 'babel.config.mjs'); |
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.
Will we still need scripts/build.mjs
?
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've kept it for now. Once all the repo PRs are merged, we can delete it in a separate PR.
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.
but these may not necessarily have babel.config.mjs yet
This is the last blocker that I am investigating currently. |
d5c148f
to
a59b6f1
Compare
a59b6f1
to
816ab59
Compare
5671f7e
to
900ef81
Compare
@Janpot All issues have been fixed. |
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.
Awesome
900ef81
to
cc3242b
Compare
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.
👍 Tested with the PR release packages, works fine.
See original PR - mui/mui-public#408 for full context and description.