By changing the display name tools like enzyme find do not work because the tester has to know if it is a connected or not component because the name changes. I vote that the name stays the same as the component has been enhanced but not really changed.
It does not _change_ the existing name. It wraps your existing component with another component that manages the updates. Wrapping it with the component of the same name but a different implementation would be confusing. cc @lelandrichardson
I just ran into this issue (not for testing with Enzyme, unrelated). I worked around it by checking for WrappedComponent but ideally it would be good to have a way to get the underlying component name in a consistent way for both connected and non-connected components.
if(exported.prototype.isReactComponent) {
var displayName = exported.displayName;
if('WrappedComponent' in exported) {
displayName = exported.WrappedComponent.displayName;
}
}
Display name is meant for debug purposes and tooling like React DevTools. Why do you rely on it?
I'm writing a tutorial for making a universal React app and I didn't want to require webpack for people trying to run the example code, so I include component.js files via script tags and use babel to process them in the browser, while still keeping the regular code like require('react') and module.exports = ComponentName in the component files (without writing if/else blocks), so I kinda 'shimmed' node.js require and module.exports like this: https://gist.github.com/firasd/684025ebbc441c67401afb2cea6fa979
I get that it's totally ridiculous in a way, will mention in my tutorial that it's only a temporary thing and readers should use webpack. So I understand your point; just wanted to mention my (admittedly brittle) workaround in case anyone ends up with this issue.
Just to briefly chime in here:
displayName in enzyme is a crutch and a hack and I would recommend avoiding it in all possible cases.Nevertheless, redux could adjust the code it uses to create it's HOC to facilitate testing (and perhaps other things) by providing a static property reference to the original (unwrapped) component. This would allow people testing with enzyme to access the component directly, and find or render it directly by reference, which is the better route.
This could look something like this:
// react-redux/connect
function connect(mapState, Component) {
var Wrapped = oldConnect(mapState, Component);
Wrapped.PureComponent = Component;
return Wrapped;
}
Then in our tests, lets say we have some component Foo which is "connected" with react-redux.
We then have another component Bar which renders the connected Foo in it's render path...
// tests
import Foo from './path/to/Foo';
import Bar from './path/to/Bar';
// Bar renders a Foo
shallow(<Bar />).contains(<Foo />); // true
// Foo's render tests
const mockReduxState = { /* ... */ };
const wrapper = shallow(<Foo.PureComponent {...mockReduxState} />);
// assert on rendered output of Foo
wrapper.find('.foo-bar').length === 1;
For the most part, I've been doing this myself manually by just having a named export in the component file (or separating the connected component into it's own file). This would look something like:
// Foo.js
class Foo extends React.Component {
// ...
}
export default connect(mapState, Foo);
export { Foo };
@lelandrichardson Thanks for chiming in!
Nevertheless, redux could adjust the code it uses to create it's HOC to facilitate testing (and perhaps other things) by providing a static property reference to the original (unwrapped) component. This would allow people testing with enzyme to access the component directly, and find or render it directly by reference, which is the better route.
I think we do this, we provide WrappedComponent.
@firasd Thanks for explaining. Indeed this is very fragile.
I think we do this, we provide WrappedComponent.
Ah, perfect. This is exactly what people should be using then!
I disagree, WrappedComponent is not a perfect solution.
@ianobermiller (FormidableLabs/radium#271 on Jul 17, 2015)
Can you imagine if every decorator did the same thing?Radium(Relay(autobound(Button)))馃憥
Or are you suggesting that react-redux is the king of React standards and every HOC project out there should adopt the WrappedComponent standard? What if other HOCs provided InnerComponent or BaseComponent as their standard method? What we end up with is chaos.
@gaearon
Display name is meant for debug purposes and tooling like React DevTools. Why do you rely on it?
No matter how you slice it, the wrapped component appears to be a change in the display name to the consuming developer(s). So I ask, why does react-redux modify displayName in the first place? If it's something that is expected for people not to rely on, why does react-redux rely on changing the displayName at all? If displayName is truly inconsequential - then the react-redux team should have no problem removing their reliance on the "change" of the displayName.
+1 for @tigerC10. 馃憤
@TigerC10 I think it is more important to educate React devs what HOCs are and are not rather than implement an intentionally misrepresentative behavior for displayName to have the library function as you might expect with a naive understanding of HOCs.
connect or any other HOC does not return a decorated component in the strictest sense (attach behavior or data to something and return it) it returns a wrapping component that proxies most component interactions to the wrappee but with some additional behavior or data. I think understanding this distinction is more important than trying to pretend decorating is happening.
I'm a strong -1 on changing displayName. the displayName of connect(whatever... is non-obfuscating and deterministic. if you want to instrument components for easier testing in enzyme then just make your own HOC who's sole purpose is to expose whatever method you want for getting a handle on your wrapped component.
The solution is better education and documentation here IMO not clever renaming of the HOC to hide that it is a HOC
Of course the display name of connect(whatever... is obfuscating - if combined with other HOCs my error messages could say
check render method on something(that(has(been(wrapped(a(million(...
And that's not very helpful - especially because longer messages get truncated in the dev tools unless you expand the call stack. Furthermore, if every HOC had their own method for fussing with the displayName of the original component we could wind up with error messages like check render method on something<that [ has( been - wrapped a {{... - or worse. To assume that the consumers of react-redux will only ever wrap their components in a single HOC is naive.
More than likely, my HOCs that are imported have been tested, and if there is a problem with the code that produces an error, it's probably not the imported HOC so much as it is a problem with my own code. It doesn't help me to know that my component has been wrapped - I'm the one that wrapped it. I know it's a wrapped component. Furthermore, one quick glance at the component code and I can see that I wrapped it.
While I don't disagree with better education and documentation - you can _still do that_ without providing a different displayName to the consuming developer's original component.
So what we have here is react-redux contributing to (not solely responsible for) the difficulty of debugging in order to "educate" about how HOCs work? Please don't patronize me. And react-redux is not interested in being a good neighbor to other libraries for debugging in the interest of some perceived "purity"?
@TigerC10 I'm sorry that you felt I meant you needed education in how HOCs work. That wasn't my intention. point noted on the difficulty of wrapping things in many HOCs.
@gnoff My apologies, the "please don't patronize me" remark wasn't intended to suggest that I felt like you were personally patronizing me - just expressing the fact that I don't expect that developers feel like displayName is truly educational.
I'm a bit confused on what the actual crux of the complaint is. Is it that you want to be able to look up components by display name during unit testing, or just that hypothetical multi-wrapped component display names would be harder to read?
Also, #395 is pretty relevant to this.
I think #395 is hilarious. "There's a problem with 'changing' the displayName - let's add a function to get the original one" instead of "Why don't we just stop 'changing' the displayName."
Most helpful comment
I disagree,
WrappedComponentis not a perfect solution.Or are you suggesting that
react-reduxis the king of React standards and every HOC project out there should adopt theWrappedComponentstandard? What if other HOCs providedInnerComponentorBaseComponentas their standard method? What we end up with is chaos.No matter how you slice it, the wrapped component appears to be a change in the display name to the consuming developer(s). So I ask, why does
react-reduxmodifydisplayNamein the first place? If it's something that is expected for people not to rely on, why doesreact-reduxrely on changing thedisplayNameat all? IfdisplayNameis truly inconsequential - then thereact-reduxteam should have no problem removing their reliance on the "change" of thedisplayName.