displayName set on forwardRef is not honored

[quantizor]

Describe the bug
In styled-components v4 beta we started decorating the forwardRef wrapper with displayName and various other statics to reduce the amount of interim component layers during rendering and preserve various library behaviors. https://github.com/styled-components/styled-components/blob/30c372cf635e0db77f74c0aaacf60deda729581c/src/models/StyledComponent.js#L249-L311

However, it doesn't seem like enzyme surfaces a displayName set on a forwardRef component.

To Reproduce
See this codesandbox: https://codesandbox.io/s/yv7l2p76q9

Expected behavior
The test expectations should work since name and displayName are set on the forwardRef node.

It would be really great if this could work for a few reasons:

1. Shallow rendering does not currently render the immediate child of ForwardRef, so if we modified our library so the wrapped child of ForwardRef had all the statics etc it still wouldn't work without diving

2. The components don't show up at all when using "find" off of "mount" since ForwardRef isn't honoring displayName

Sidebar: I really wish ref forwarding was implemented as a decorator or class mixin... sigh.

[ljharb]

Hmm, does react itself respect .displayName on a forwardRef?

[quantizor]

@ljharb It doesn't in the devtools as of right now... but it's a weird case since it's a metacomponent thingy and doesn't have a renderable

[quantizor]

cc @gaearon

[ljharb]

If it doesn't show up in the dev tools, then I wouldn't expect it to make any sense to have a displayName - typically only custom components have displayNames, and forwardRef produces a special kind of component that's not a class or an SFC.

[quantizor]

Yeah, I’m not really sure what to do about it honestly. Usage of the ref forwarding API is incredibly awkward, but the functionality is super compelling.

…

On Fri, Sep 7, 2018 at 5:58 PM Jordan Harband ***@***.***> wrote: If it doesn't show up in the dev tools, then I wouldn't expect it to make any sense to have a displayName - typically only custom components have displayNames, and forwardRef produces a special kind of component that's not a class or an SFC. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1810 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAiy1vD5c-j1-midQWwLTBpnVl69XG4wks5uYvorgaJpZM4WflEw> .

[gaearon]

If DevTools doesn't show it that sounds more like a bug with DevTools that we can fix.

[gaearon]

Do React warnings show it?

[quantizor]

I'll experiment and get back to you

[quantizor]

@gaearon I don't believe it does:

Users/bear/code/homepage/node_modules/react-error-overlay/lib/index.js:2172 Warning: Failed prop type: The prop `fluid.aspectRatio` is marked as required in `Image`, but its value is `null`.
    in Image (created by Context.Consumer)
    in BaseStyledComponent (created by ForwardRef)     <- that should be a named component
    in article (at PostExcerpt/index.js:24)
    in PostExcerpt (at PostList/index.js:13)
    in div (created by Context.Consumer)
    in BaseStyledComponent (created by ForwardRef)

[quantizor]

@gaearon this will be a problem with react-native too since they started doing the same thing we are: facebook/react-native@e708010#diff-e250436371d8e58356be701ad28b37caR268

[quantizor]

Made a PR against React to support this: facebook/react#13615

[jackson-sandland]

@probablyup I'm running into the issue you're describing.
I've confirmed the changes from this PR are in my local react src.
I've created a Button that uses forwardRef.
I've set a displayName as a static property in that file.
Button.displayName = "Button"

When using find in an enzyme test - the static property displayName is not recognized.
ForwardRef is recognized.
let component = shallow(<Form {...props} />).find("ForwardRef"); <-- component found
let component = shallow(<Form {...props} />).find("Button"); <-- component not found

[ljharb]

@jackson-sandland what version of react and enzyme and enzyme adapter are you using?

[jackson-sandland]

@ljharb
react@16.5.2
enzyme@3.6.0
enzyme-adapter-react-16@1.5.0
enzyme-adapter-utils@1.8.0

I figured out how to set the displayName but it's the Higher Order Component version of the displayName e.g. higherOrderWrapper(wrappedComponent).
Is that the best option we've got?
Can we actually get a find by the displayName of the wrappedComponent?

Just found a comment by @lencioni over at AirBnb saying finding by component reference instead of string is preferred.
airbnb/react-with-styles#32 (comment)
This approach avoid issues with displayName and HOC's but it would be great to be able to use the string displayName so we don't have to update this everywhere.

[ljharb]

Finding by reference is preferred; so in that case, you can find by the wrapped component constructor itself.

Setting the displayName should make it show up in .debug(), but it may still be a bug that it's not findable.

[jackson-sandland]

@ljharb
It must still be a bug.
Here's the output of wrapper.debug():

<div className="container">
  <ForwardRef(Button) className="btn">
    Text here
  </ForwardRef(Button)>
</div>

The approach that produced the above result is:

const Button = (props, ref) => {
  const {
    className,
  } = props;
  return (
    <button
      className={className}
      ref={ref}
    >
      <span>{children}</span>
    </button>
  );
};

const buttonWithRefForwarding = forwardRef(Button);
buttonWithRefForwarding.displayName = "Button";

export default buttonWithRefForwarding;

The code below worked in the unit tests where Button is a ChildComponent, but did not when the Button was bing shallow rendered in it's own unit tests.
Didn't get a chance to make sure it still worked in the browser after the tests blew up.

const Button = function Button(Component) {
  const {
    className,
  } = props;
  const returnedButton = () => (
    <button 
      className={className}
      ref={ref}
      >
        <span>{children}</span>
      </button>
  );

  function forwardRef(props, ref) {
    return returnedButton;
  }

  return React.forwardRef(forwardRef);
}

[ljharb]

@jackson-sandland would you mind filing a new issue covering all this? I'll be happy to take a look at it ASAP.