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:
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
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_.
Hmm, does react itself respect .displayName on a forwardRef?
@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
cc @gaearon
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.
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 notifications@github.com
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
https://github.com/airbnb/enzyme/issues/1810#issuecomment-419587905, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAiy1vD5c-j1-midQWwLTBpnVl69XG4wks5uYvorgaJpZM4WflEw
.
If DevTools doesn't show it that sounds more like a bug with DevTools that we can fix.
Do React warnings show it?
I'll experiment and get back to you
@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)
@gaearon this will be a problem with react-native too since they started doing the same thing we are: https://github.com/facebook/react-native/commit/e708010d18f938e2d6b6424cfc9485d8e5dd2800#diff-e250436371d8e58356be701ad28b37caR268
Made a PR against React to support this: https://github.com/facebook/react/pull/13615
@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
@jackson-sandland what version of react and enzyme and enzyme adapter are you using?
@ljharb
[email protected]
[email protected]
[email protected]
[email protected]
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.
https://github.com/airbnb/react-with-styles/issues/32#issuecomment-255425391
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.
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.
@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);
}
@jackson-sandland would you mind filing a new issue covering all this? I'll be happy to take a look at it ASAP.
Most helpful comment
Made a PR against React to support this: https://github.com/facebook/react/pull/13615