Skip to content

[core] refactor: remove manual displayName #46147

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

romgrk
Copy link
Contributor

@romgrk romgrk commented May 14, 2025

Follow-up of mui/base-ui#1920
Depends on mui/mui-public#329

closes #44067

@romgrk romgrk added core Infrastructure work going on behind the scenes enhancement This is not a bug, nor a new feature labels May 14, 2025
@romgrk romgrk requested a review from a team May 14, 2025 19:09
@mui-bot
Copy link

mui-bot commented May 14, 2025

Netlify deploy preview

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

@mui/material: parsed: +0.12% , gzip: +0.10%

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 489a0b4

@Janpot
Copy link
Member

Janpot commented May 15, 2025

🤔 Where does the bundle size increase come from? Shouldn't these be wrapped in a process.env.NODE_ENV check. The bundle size checker should take it into account.

@romgrk
Copy link
Contributor Author

romgrk commented May 15, 2025

It's all wrapped, here is the output of diff -bur build-master build-this-pr for material-ui: https://gist.github.com/romgrk/98b47d15d498704bd61f89082374c36d. Don't know how the bundle size checker works, can't say anything about that.

@Janpot
Copy link
Member

Janpot commented May 15, 2025

The current implementation runs webpack with minifier on a virtual module that imports the package and compares the result with merge base. Interestingly it seems to minify differently. Looks like in the baseline it omits variable declaration in the function body to create a global var, and in this PR it explicitly creates those const in global scope. Quite odd, maybe whitespace related in terser. https://gist.github.com/Janpot/2d3156e4ee4302687e29ae848f230c06

For now it probably makes most sense to not run this plugin on the tests.

@romgrk
Copy link
Contributor Author

romgrk commented May 19, 2025

I've disabled the plugin in test. This is ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes enhancement This is not a bug, nor a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants