fix(core / styled): fix wrap the display name in styled and withEmotionCache #1692
Conversation
🦋 Changeset is good to goLatest commit: eea168b We got this. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report
|
|
cc @Andarist |
| })` | ||
|
|
||
| const injectDisplayName = Component => { | ||
| Component.displayName = Component.displayName || displayName |
There was a problem hiding this comment.
there is no way for the Component to have a .displayName as it's always the anonymous function
| @@ -78,8 +92,8 @@ let createStyled: CreateStyled = (tag: any, options?: StyledOptions) => { | |||
| } | |||
|
|
|||
| // $FlowFixMe: we need to cast StatelessFunctionalComponent to our PrivateStyledComponent class | |||
| const Styled: PrivateStyledComponent<P> = withEmotionCache( | |||
| (props, context, ref) => { | |||
| const Styled: PrivateStyledComponent<P> = withEmotionCache<P>( | |||
There was a problem hiding this comment.
instead of doing this injectDisplayName I think it could be more readable if we could just split this into separate statements, like this:
const render = (props, context, ref) => {}
render.displayName = displayName
const Styled = withEmotionCache(render)That being said - shouldn't we also make this displayName in here a little bit more distinct? Right now 2 component will receive the very same displayName from what I can see (the one being this render here and the other one being Styled)
There was a problem hiding this comment.
Right now 2 component will receive the very same displayName from what I can see
In my practice, I found that Styled.displayName has no effect.
Origin Styled.displayName got the result as the screenshots shown above.
There was a problem hiding this comment.
But first, i wrote codes like yours comment, but i found some problems that
-
its less semantic than
injectDisplayNameToComponent(Component, displayName)func -
if you want use the same code to solve same problems with other codes where
withEmotionCacheused, it will make lots of code changed;On the other hand
injectDisplayNamefunc is reusable logic if export and necessary
There was a problem hiding this comment.
I found that Styled.displayName has no effect.
i guess, its due to forwardRef in HOC
There was a problem hiding this comment.
i guess, its due to forwardRef in HOC
yes, source code at forwardRef.js#L46
seems do not accept Component.displayName after forwardRef return;
some POC codes and results here:
| @@ -32,6 +45,9 @@ let withEmotionCache = function withEmotionCache<Props>( | |||
| </EmotionCacheContext.Consumer> | |||
| ) | |||
| } | |||
|
|
|||
| render.displayName = getDisplayName<Props>(func) | |||
There was a problem hiding this comment.
does func have a name under any circumstances? I think we always pass anonymous functions to it
There was a problem hiding this comment.
Not before, so I made injectDisplayName in styled-base to inject displayName before withEmotionCache,
the func still has no name property yet, but maybe post in the future in withEmotionCache, then withEmotionCache also ability to use name.
|
I try to fix it in react devtools in facebook/react#17613 |
|
Hm, this seems like some sort of regression in devtools. I've found a PR supposedly fixing this: https://github.com/facebook/react-devtools/pull/1154/files |
|
yep, I think it's actually a problem in React DevTools. |
|
Thanks for review, I have fixed the real reason in facebook/react#17613 |
|
No problem, I'm glad that you have fixed this in devtools ❤️ |
What: display correct styled component's name in React DevTools
Why:
@emotion/styledusewithEmotionCachefunction which is a HOC but not wrap a displayName.So in React DevTools, it will be always show only
renderstring as component name.Before wrap displayName:
How:
Follow React displayName docs, inject
displayNamebefore and intowithEmotionCache.After wrap displayName:
Checklist: