@material-ui/icons 4.11.2 no longer has unique displayNames on icons according to enzyme .name()

[heath-freenome]

Prior to 4.11.2 when I used an enzyme wrapper to get the name() of an icon component, it returned the name of each component properly (i.e. the Error icon return ErrorIcon, the Add icon returned AddIcon, etc.). Now all icons return the same name: Memo([object Object])

See the test file in an example code sandbox

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

The name of all icons in 4.11.2 is Memo([object Object])

Expected Behavior 🤔

The name of every icon in 4.11.2 is the same as they were in 4.9.1

Steps to Reproduce 🕹

Steps:

1. Using this codesandbox instance, see the tests working properly with 4.9.1
2. Using this codesandbox instance, see the same tests failing with 4.11.2

Context 🔦

My tests for my applications are checking to see if I have the proper icon being rendered using the name() function on an enzyme wrapper, with this upgrade my tests are breaking.

Your Environment 🌎

An example of a functioning code is found in this code sandbox with @material-ui/icons@4.9.1: https://codesandbox.io/s/brave-galois-hm1mf
An example of the breaking code is found in this code sandbox with @material-ui/icons@4.11.2: https://codesandbox.io/s/nostalgic-brahmagupta-djl98

Tech	Version
Material-UI	v4.11.2
React	16.14.0
Browser	N/A
TypeScript	N/A
etc.

[oliviertassinari]

This is related to #21134. Note that we discourage using enzyme. I believe #22634 correctly solve this problem.

[oliviertassinari]

@eps1lon Do we still need #21134 on next, considering React the devtool was fixed (I imagine)?

[heath-freenome]

This is related to #21134. Note that we discourage using enzyme. I believe #22634 correctly solve this problem.

Why would you discourage using enzyme? It's one of the most popular react test frameworks out there

[heath-freenome]

Regardless of being discouraged to use enzyme, is the data-testid available in 4.11.2 of the icons?

[heath-freenome]

Ok, I checked the source code, it is... sadly, it requires that I mount() the icon instead of shallow rendering it. Why remove the displayName on the icons, but keep it on the regular components? I would suggest that the proper fix for #21134 would have added the displayName on the inner component without removing it from the outermost one. And since this change could potentially break more than just my tests... is it possible to add displayName back on the outermost component?

[oliviertassinari]

Why would you discourage using enzyme? It's one of the most popular react test frameworks out there

enzyme is no longer the most popular one, and has little to no traction: https://npm-stat.com/charts.html?package=%40testing-library%2Freact&package=enzyme. We have migrated most of our tests, mostly because removing shallow tests made us more efficient and mount test allows too much to hook into private component APIs.

it requires that I mount() the icon instead of shallow rendering it

If this works, great! We don't encourage shallow tests either, quite the opposite.

---

Let's wait to get Sebastian's perspective on this matter. A good solution might be to revert #21134 as react dev tools are meant to be kept up to date.

[eps1lon]

I don't know how enzyme's name() behaves these days with forwardRef and memo. Especially across various versions of React.

The usual advise still stands: Don't use enzyme unless you're following a testing approach that doesn't rely on component internals. Either by following the approach explained in https://testing-library.com/docs/react-testing-library/intro or actually using @testing-library/react.

Other than that I recommend rolling back to the previous version of @material-ui/icons.

[mejackreed]

FWIW, we have the same issue in our projects test suite. We rely on enzyme (I know I know), and SvgIcon displayNames. Will look at potential solutions as well. ProjectMirador/mirador#3366

[heath-freenome]

I just locked down icons to the previous release and just was done with it. I would consider not applying

@eps1lon Do we still need #21134 on next, considering React the devtool was fixed (I imagine)?

I would highly encourage not applying it to next

[oliviertassinari]

@heath-freenome Also, could it be an issue with how enzyme computes the name?

[heath-freenome]

@heath-freenome Also, could it be an issue with how enzyme computes the name?

#21134 removed the .displayName on the outer component... which is what enzyme uses to compute the name()

[mejackreed]

I'm looking at an enzyme solution right now. Looking at: https://github.com/facebook/react/pull/18925/files

"should honor a inner displayName if set on the wrapped function"

Seems like enzyme should support this.

[mejackreed]

I think this is on the right track to at least better compute the displayName the "enzyme way" enzymejs/enzyme#2482

[KeitelDOG]

From now on, I test Material UI Icons with the Icon Component directly, not with name anymore, like:

import SearchIcon from '@material-ui/icons/Search';

// ...

  it('should render an Icon for search', () => {
    expect(wrapper.find(SearchIcon)).toHaveLength(1);
  });

Also I'm using NextJS.
Guide: https://medium.com/fredwong-it/react-enzyme-expect-material-ui-icon-memo-object-object-a988c92f6d17