[icons] Remove SvgIcon data-testid in production

[Janpot]

Removes the default data-testid prop when in production. Adds a section to the migration guide and remove the docs that describe this behavior.

[mui-bot]

Netlify deploy preview

• docs/data/material/components/icons/icons.md
• docs/data/material/migration/upgrade-to-v7/upgrade-to-v7.md

@material-ui/core: parsed: -0.07% 😍, gzip: -0.13% 😍

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against dd6fa25

[EfstathiadisD]

Terrible! Terrible folks! What on earth are y folks doing over there?

[Janpot]

What on earth are y folks doing over there?

@EfstathiadisD Trimming unnecessary bloat from production code, these test ids end up in server rendered html and potentially clash with user defined test ids.

The component still spreads props on the svg element, so a user defined testid will still end up there. Wouldn't that suffice for your use-case? Could you elaborate a bit more on why you think this should be kept?

[EfstathiadisD]

What on earth are y folks doing over there?

@EfstathiadisD Trimming unnecessary bloat from production code, these test ids end up in server rendered html and potentially clash with user defined test ids.

The component still spreads props on the svg element, so a user defined testid will still end up there. Wouldn't that suffice for your use-case? Could you elaborate a bit more on why you think this should be kept?

It was one of the most efficient ways to write tests!

We all had 1 way to test them, and now people are gonna start renaming it how they like.

I think options hurt the ecosystem more than strictness. I understand that it might not be beneficial but consider an approach where this Icons can have predefined data-testId.

Like a flag or something. And I suppose this goes to V7 as a breaking change. In our repo it about 35 instances of this.

And our apps aren't huge! Just IMHO!

[Janpot]

Like a flag or something.

Would it work for you if we add the test id only when process.env.NODE_ENV is not 'production'?

[mnajdova]

Add the test id only when process.env.NODE_ENV is not 'production'?

I think this is a good compromise, I can't imagine anyone wanting to have these in production.

[EfstathiadisD]

Like a flag or something.

Would it work for you if we add the test id only when process.env.NODE_ENV is not 'production'?

Yeah. That makes lots of sense! Thank You!!🙏