[core] Remove outdated Babel plugins

ZeeshanTamboli commented 1 week ago

[x] I have followed (at least) the PR section of the contributing guide.

Part of #40958.

With the updated browser support, particularly for the new Safari browser, it seems we no longer require these plugins. Previous attempted PRs mentioned in #40958.

Additionally, it appears that the assumptions feature of Babel is also not needed. This closes #37461.

The plugins are now included by default in the latest version of @babel/preset-env. They are applied based on the browsers listed in browserslist, with Babel maintaining a mapping of versions for each plugin here.

browserstack-force run for Safari: https://app.circleci.com/pipelines/github/mui/material-ui/128548/workflows/5eb395f3-c3f8-4e63-bdd3-f51a5e790419/jobs/693108

Bundle size reduction.

mui-bot commented 1 week ago

Netlify deploy preview

https://deploy-preview-42140--material-ui.netlify.app/

@material-ui/core: parsed: -3.86% :heart_eyes:, gzip: -3.80% :heart_eyes: @mui/joy: parsed: -3.00% :heart_eyes:, gzip: -3.12% :heart_eyes: @material-ui/lab: parsed: -2.90% :heart_eyes:, gzip: -3.08% :heart_eyes: TextField: parsed: -3.63% :heart_eyes:, gzip: -2.95% :heart_eyes: @material-ui/unstyled: parsed: -2.86% :heart_eyes:, gzip: -2.70% :heart_eyes: @mui/joy/Autocomplete: parsed: -2.18% :heart_eyes:, gzip: -2.19% :heart_eyes: Autocomplete: parsed: -2.32% :heart_eyes:, gzip: -2.10% :heart_eyes: SpeedDialAction: parsed: -2.18% :heart_eyes:, gzip: -1.95% :heart_eyes: SwipeableDrawer: parsed: -2.74% :heart_eyes:, gzip: -2.23% :heart_eyes: @mui/joy/Tooltip: parsed: -2.40% :heart_eyes:, gzip: -2.42% :heart_eyes: @mui/joy/Select: parsed: -2.01% :heart_eyes:, gzip: -1.45% :heart_eyes: @mui/joy/Checkbox: parsed: -2.66% :heart_eyes:, gzip: -2.76% :heart_eyes: Popover: parsed: -2.71% :heart_eyes:, gzip: -2.02% :heart_eyes: @mui/joy/Textarea: parsed: -2.77% :heart_eyes:, gzip: -2.72% :heart_eyes: @mui/joy/ChipDelete: parsed: -2.38% :heart_eyes:, gzip: -2.51% :heart_eyes: @mui/joy/ModalClose: parsed: -2.31% :heart_eyes:, gzip: -2.48% :heart_eyes: @mui/joy/Input: parsed: -2.69% :heart_eyes:, gzip: -2.71% :heart_eyes: @mui/joy/MenuButton: parsed: -2.32% :heart_eyes:, gzip: -2.24% :heart_eyes: Drawer: parsed: -2.53% :heart_eyes:, gzip: -1.88% :heart_eyes: @mui/joy/Radio: parsed: -2.51% :heart_eyes:, gzip: -2.69% :heart_eyes: and 217 more changes

Bundle size report

Details of bundle changes (Toolpad) Details of bundle changes

Generated by :no_entry_sign: dangerJS against f84b8c0456c645cff33f4ef3ee2af5ee6f3bfe58

Janpot commented 1 week ago

packages/material-ui/material-ui.production.min.js: parsed: +18.78% , gzip: +5.84%

Any idea where this 18% increase comes from?

ZeeshanTamboli commented 1 week ago

packages/material-ui/material-ui.production.min.js: parsed: +18.78% , gzip: +5.84%

Any idea where this 18% increase comes from?

Not sure. It seems to be related to the umd build. Do you have any ideas on how we can compare the actual build size of master versus the build on this branch for umd? It's possible that our code in dangerFile.ts or contributor-dashboard-legacy code in mui-public is miscalculating it.

Janpot commented 1 week ago

You could try running

 pnpm --filter @mui/material build:umd

on master and copy the packages/mui-material/build/umd/material-ui.production.min.js somewhere else, then run the same command on this branch and then git diff both files . Maybe that will give a clue on what was added extra by this branch?

ZeeshanTamboli commented 1 week ago

@Janpot Yesterday, I spent quite some time checking and debugging.

I compared the development files instead of the production ones because the production file is unreadable, likely due to terser. Here's what I did:

Checked out the next branch.
Ran pnpm install to get the Babel plugins.
Built the UMD bundle using pnpm --filter @mui/material build:umd.
Copied the material-ui.development.js file and renamed it to next-material-ui.development.js.
Checked out this branch.
Ran pnpm install and built the UMD bundle again.
Copied the material-ui.development.js file and renamed it to babel-branch-material-ui.development.js.
Compared both files using git diff --no-index --word-diff next-material-ui.development.js babel-branch-material-ui.development.js.

Here's a sample diff which is the same till the end:

https://github.com/mui/material-ui/assets/20900032/01789cb9-cf46-4df3-82af-5f36e191f0bd

What I noticed is that it's using objectSpread (which is defined multiple times) instead of _extends, resulting in a larger bundle size. This change is due to the @babel/plugin-transform-object-rest-spread plugin removal. When loose: true, it uses the _extends helper (https://babeljs.io/docs/babel-plugin-transform-object-rest-spread#loose), which was the case in the next branch.

What I don't understand is why rollup's babel is using the object-spread plugin in the first place? Is it that it is not considering the .browserslistrc file and checking the browser versions?

DiegoAndai commented 1 week ago

@ZeeshanTamboli I plan to work on the UMD removal hopefully this week or next, but if you want to unblock this PR by removing it yourself, feel free to go ahead.

oliviertassinari commented 1 week ago

How about we open a new issue for this? https://pagespeed.web.dev/analysis?url=https%3A%2F%2Fdeploy-preview-42140--material-ui.netlify.app%2F

This can be reproduced too in HEAD. I imagine that Lighthouse uses Cannot call a class as a function to detect https://babeljs.io/docs/babel-plugin-transform-classes. Yes, bingo: https://github.com/GoogleChrome/lighthouse/blob/369979f498bd6560127e10476edffb264d4fa3b9/core/audits/byte-efficiency/legacy-javascript.js#L301C22-L301C55.

Seeing this on https://deploy-preview-42140--material-ui.netlify.app/ is wrong:

We should at minimum go into loose mode: https://github.com/vercel/next.js/blob/5ff2731c589692ed86379f876a38e1ca46f5761e/packages/next/src/build/babel/preset.ts#L161.

I also left the feedback in https://github.com/vercel/next.js/issues/65540.

ZeeshanTamboli commented 1 week ago

@ZeeshanTamboli I plan to work on the UMD removal hopefully this week or next, but if you want to unblock this PR by removing it yourself, feel free to go ahead.

@DiegoAndai Thanks. I've already begun working on this yesterday. Here's the PR: https://github.com/mui/material-ui/pull/42172. Feel free to push any changes if needed.

How about we open a new issue for this?

@oliviertassinari Do we need to make any changes outside of this PR? I suppose it's all related to Next.js with the new issue you created there.

Janpot commented 1 week ago

Given the Next.js team's track record with the maintenance of the babel setup in Next.js I would assume we'll be using their SWC setup before this gets fixed 😄.

Btw, yesterday, on top of this PR on the X repo I did a few quick tests and I could just remove the babel config without breaking their build. Page compilation in dev mode went from ~8s to ~4.5s on my machine. I couldn't enable turbopack due to our usage of esmExternals.

DiegoAndai commented 1 week ago

@ZeeshanTamboli would it be possible to come up with a detailed list of Safari 12 features that will stop working on v6? This is so users can adjust accordingly.

ZeeshanTamboli commented 1 week ago

would it be possible to come up with a detailed list of Safari 12 features that will stop working on v6? This is so users can adjust accordingly.

@DiegoAndai I'm not familiar with the specific features of Safari 12, but whatever was available in Safari 12 will be carried forward to the newly supported Safari 15.6 version. We can document the list of supported browser versions, as done in the Migrating to v5 docs, but that should be done in a separate PR.

ZeeshanTamboli commented 1 day ago

@Janpot @DiegoAndai Now that #42172 is merged, this PR is ready for review.

mui / material-ui

[core] Remove outdated Babel plugins #42140

Netlify deploy preview

Bundle size report