-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[code-infra] Reconfigure react-remove-properties
babel plugin
#45218
base: master
Are you sure you want to change the base?
Conversation
Netlify deploy previewhttps://deploy-preview-45218--material-ui.netlify.app/ Bundle size report |
This reverts commit 9fd6395.
data-testid
from our builds
data-testid
from our buildsreact-remove-properties
for the docs
react-remove-properties
for the docsreact-remove-properties
babel plugin
History:
Looking at the rest of the codebase, I only see the pickers components that depend on |
Couldn't they just provide the |
This reverts commit aa647e2.
data-mui-test
was removed in [test] Remove data-mui-test from tests #23498. In the main babel config this meansbabel-plugin-react-remove-properties
was a no-op. We can't really rely on it because the icons have adata-testid
as documented public API.data-testid
pops are still in the docs. They're not removed from the icons as we're excluding babel on them. Here we can keepbabel-plugin-react-remove-properties
and reconfigure it to operate on^data-test
.babel-plugin-macros
plugin which we don't use anymore. I didn't realize there was another babel config the docs folder. Looks like this was working because it's pulled in as a peer dependency by another package.data-testid
from markdown generated HTML.Noticed this in #45208
Seeing about 4k occurrences of
data-testid
in the docs due to it being baked in the icons. We're not transpiling the icons package for performance reasons, and I imagine many with us. For some obscure reason we made this a public API, so we can't just remove them on a minor. Adding as an idea here.We weren't seeing the warning in Next.js because of vercel/next.js#75682