nkbt/react-collapse

Set a displayName for the component

Closed this issue · 7 comments

ivarni commented

This is really a quality of life thing more than anything else but giving the component a displayName would make it easier to use e.g. react dev tools (which on the latest version just refers to the <Collapse> wrapper as <Unknown>) or testing with e.g. enzyme which right now refers to it as <Component>.

The solution would be to set a displayName when creating the component with createReactClass here

export const Collapse = createReactClass({
  displayName: 'Collapse',
  propTypes: { ... }
  ...
});

I'm happy to make a PR for it if it's something you'd agree to add.

nkbt commented

Hm, it used to be working fine. Probably babel transform added displayName automatically based on the variable name.

I def don't mind a PR for this, but would be nice to know if that was the case and if it could be solved with babel instead?

nkbt commented

Actually babel/babel#5554

  • 👍1
ivarni commented

One of our unit tests started breaking with 2.3.4 because of this so it might be related to the recent change to using create-react-class. I can dig into it a bit more.

ivarni commented

Ah yeah, that seems quite relevant to this :)

nkbt commented

I feel like it would be better to wait until babel is updated. Unless it is too difficult... I'm OK with adding displayName

tbh, it is a bit strange to rely on component name in unit tests, though I don't know your use-case so it might be important to have it.

ivarni commented

Yeah, it's enzyme's API that has functionality for locating components in the rendered tree to check if they've been rendered with appropriate props but I see your point and I'm totally fine with waiting for babel on this one. Thanks for the quick replies :)

nkbt commented

👍