-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[SvgIcon] Add displayName in react-devtools #21134
Conversation
Details of bundle changes.Comparing: 90babbd...f871c76 Details of page changes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great. That was annoying me for quite a while but I didn't realize it works on the inner component.
Let's keep this in for backwards compat since the next react minor should fix this.
@gndplayground Very nice, thanks! |
</SvgIcon> | ||
)), | ||
const Component = (props, ref) => ( | ||
<SvgIcon data-mui-test={`${displayName}Icon`} ref={ref} {...props}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really related to the diff in this PR, but what do we need memo
for exactly? Was it added to prevent unnecessary re-renders or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's one of the only components that doesn't accept a react element as a prop, so one of the few spots where memo
can work on the library. Do we have a benchmark that shows the win on a real-life use case? No.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's most likely worse for perf since SvgIcon almost never has an expensive tree on which we want to bail out. But now we have it so somebody might rely on it.
I checked in the devtool I cannot see Icon display name.
Before
After
Not sure if it related to the PR here facebook/react#17274