Skip to content
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

Improve wrapping of component name with displayName #101

Merged

Conversation

SamLeatherdale
Copy link
Collaborator

I noticed in my project some of my wrapped components were still displaying as Anonymous in React DevTools.

This is because they are using .displayName instead of .name for their name.
If you see React displayName docs, this describes how the displayName property is used. This is not generally hand set but is often set for you by build tools, especially when using arrow functions.

I noticed in my testing it wasn't enough to just use the displayName property for getting the name, but also setting the name on the resulting component, hence the change in two places.

This makes my DevTools much easier to read now.

@SamLeatherdale
Copy link
Collaborator Author

In the docs for Wrapping Display Name they actually recommend using displayName as higher priority than name:

function getDisplayName(WrappedComponent) {
  return WrappedComponent.displayName || WrappedComponent.name || 'Component';
}

However I put displayName as lower priority in my PR to preserve existing behaviour as much as possible.
Let me know if you'd like them swapped around to match the recommended order.

@garronej
Copy link
Owner

garronej commented Aug 15, 2022

Hi @SamLeatherdale,
Thank you for the PR! sorry I am only reviewing it now, I am on vacation.

@garronej
Copy link
Owner

Hi @SamLeatherdale,
Thank you for your PR, it's a valuable contribution!
Would you change the base branch of this PR from main to v4.
The new version will be released soon, I'm not adding new features to v3.

BTW, I gave you write access on the project: https://github.com/garronej/tss-react/invitations

@SamLeatherdale SamLeatherdale changed the base branch from main to v4 August 16, 2022 14:19
@SamLeatherdale
Copy link
Collaborator Author

I went ahead and updated the order of retrieving the name as per the React docs.
Let me know if you're happy for me to go ahead and merge it 😃

@SamLeatherdale SamLeatherdale merged commit 0e5f315 into garronej:v4 Aug 17, 2022
@SamLeatherdale SamLeatherdale deleted the fix/improve-name-wrapping branch August 17, 2022 02:14
garronej added a commit that referenced this pull request Aug 18, 2022
garronej added a commit that referenced this pull request Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants