-
Notifications
You must be signed in to change notification settings - Fork 47.2k
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
Memoized components should forward displayName #15207
Comments
Sure, happy to take a PR. |
@pbondoer Did you start looking into this already ? |
@LorenzoOpWerk Yes, just haven't had a lot of free time to finish up. |
@pbondoer I'm not sure this is an issue with react but rather with enzyme. I don't know how jest serialized enzyme wrappers but I suspect it uses Find by display name might either be another issue or resolved with enzymejs/enzyme#2081 Though I did submit a PR related to display name behavior with memo components: #15313 |
I don't think this is a test renderer/enzyme issue. Astonishingly React.memo in normal usage does not appear to pass on |
Same issue appears when using it with React devtools. Wrapped components show up as |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution. |
Should not be closed. |
I peeked into the memo source. This is a bit more complex than we thought. Normally, most HOCs return a component and set a displayName based on However So there's no simple "wrap the displayName like other HOCs" because the result is not a component. It's an informational object that React understands how to process and is closer to the the ReactElement returned by createElement than it is to a component. I see two potential solutions. Solution 1: Hack a displayName inThe quick and dirty fix would be to modify This feels like a hack because the memo object is technically not a component and would be equivalent to adding a displayName to Solution 2: Expect wrappers to handle thisThe alternative is to expect that any HOC wrappers will handle memo themselves. So when they see the However if this is the plan I do not think it would be fair to make 3rd party libraries all handle this themselves. React actually already has this internally. getComponentName.js contains both a For this solution
This is basically #14319. |
This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment! |
bump |
This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment! |
Still an issue |
This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment! |
bump |
I have a bit of free time these holidays, is this being worked on elsewhere / fixed by something else or is there anything I can do to aid in its resolution? |
Do you want to request a feature or report a bug?
I'd like to report a bug.
What is the current behavior?
First of all, thanks for the great work on fixing #14807. However there is still an issue with the current implementation.
React.memo
does not forward displayName for tests. In snapshots, components display as<Component />
and string assertions such as.find('MyMemoizedComponent')
won't work.What is the expected behavior?
React.memo
should forward displayName for the test renderer.Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
N.B. - Potentially related to #14319, but this is related to the more recent changes to support
memo
in the test renderer. Please close if needed, I'm quite new here!I'd be happy to submit a PR if the issue is not too complex to look into 😄
The text was updated successfully, but these errors were encountered: