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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support setting Context displayName in `@babel/plugin-transform-react-display-name` #11241

Open
kentcdodds opened this issue Mar 11, 2020 · 6 comments

Comments

@kentcdodds
Copy link
Member

@kentcdodds kentcdodds commented Mar 11, 2020

Feature Request

  • I would like to work on this feature!
  • I have bandwidth to work on this feature in the near future. 馃檨

Is your feature request related to a problem? Please describe.

Here's an example of React DevTools with a few context providers and consumers:

image

You'll notice that the context consumer and provider rendered under ThemeProvider have generic Context.Consumer and Context.Provider display names, but there's also a Router.Provider and Router.Consumer in there which are much more clear.

They accomplish this by setting the context's displayName value.

Here are some test cases:

import React from 'react'

const MyContext = React.createContext()
MyContext.displayName = 'MyContext'

Describe the solution you'd like

I'd like @babel/plugin-transform-react-display-name to support automatically setting the displayName property for contexts.

Describe alternatives you've considered

I can set it manually. Not desirable. Or I could write my own babel plugin, but then others wouldn't benefit from it if they either don't know about the plugin or can't install it. Also, this seems to make a lot of sense to be included here to me.

Teachability, Documentation, Adoption, Migration Strategy

No need, it'll just magically start working and people will be pleasantly surprised.

@kevin940726

This comment has been minimized.

Copy link

@kevin940726 kevin940726 commented Mar 11, 2020

I would love to help :) !

However, I think @babel/plugin-transform-react-display-name only supports assigning displayName to components created by createClass? I'm not sure if it should be the right target package we should build this into? Maybe I'm missing something 馃

@kentcdodds

This comment has been minimized.

Copy link
Member Author

@kentcdodds kentcdodds commented Mar 11, 2020

Yes, I believe that's true. That said, there's nothing in the name to suggest that all it handles and considering it's already used by projects which would want this feature I think it's an acceptable place to put this.

@kevin940726

This comment has been minimized.

Copy link

@kevin940726 kevin940726 commented Mar 11, 2020

@kentcdodds Fair enough 馃憤! I'm just curious then why babel-plugin-add-react-displayname isn't also part of this?

@kentcdodds

This comment has been minimized.

Copy link
Member Author

@kentcdodds kentcdodds commented Mar 11, 2020

I'm also curious about that. I definitely prefer the trade-off of a few extra bytes to have a better production debugging experience.

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Mar 11, 2020

Oh wow, I had no idea that our plugin was so outdated.

@roncohen Would you be open to adding this feature to babel-plugin-add-react-displayname, and then merging babel-plugin-add-react-displayname to the official Babel plugin?

@existentialism

This comment has been minimized.

Copy link
Member

@existentialism existentialism commented Mar 11, 2020

Related: #8748

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can鈥檛 perform that action at this time.