Javascript: [React] Add guidelines for setting displayName on higher-order components

[lencioni]

Higher-order components are functions that take a Component as an argument and return a new Component that renders the passed-in Component. When doing this, it is nice to set the displayName on the generated Component to make it obvious (e.g. in dev tools) to understand what is going on. I prefer the convention of higherOrderComponent(WrappedComponent).

For example, if I have a higher-order component called withFoo and a component that is wrapped by this called Bar:

export default withFoo(Bar);

the displayName on the generated component would be set to withFoo(Bar), e.g.:

export default function withFoo(Component) {
  function WithFoo(props) {
    // TODO: do something special here.
    return <Component {...this.props} />;
  }

  WithFoo.displayName = `withFoo(${Component.displayName || Component.name}`;
  return WithFoo;
}

[ljharb]

This seems like a reasonable thing to add to the React guide (although imo HOCs should generally be avoided in favor of composition, when possible)

[lencioni]

Agreed!

[jeffcarbs]

@lencioni - I had a question about this pattern when using https://github.com/airbnb/enzyme (I'm assuming you use enzyme because you're an airbnb member).

Normally you can pass the component's name as a string as a selector. Imagine this scenario:

const ChildComponent = () => <div />
const ParentComponent = () => <ChildComponent />

describe('ParentComponent', () => {
  it('renders ChildComponent (select by class)', function() {
    const wrapper = shallow(<ParentComponent />)

    expect(wrapper.is(ChildComponent)).to.be.true
  })

  it('renders ChildComponent (select by class name)', function() {
    const wrapper = shallow(<ParentComponent />)

    expect(wrapper.is('ChildComponent')).to.be.true
  })
})

This all works fine.

Now imagine ChildComponent needs some added functionality, you decide to decorate it:

function withFoo(WrappedComponent) {
  function WithFoo(props) {
    // TODO: do something special here.
    return <WrappedComponent {...props} />
  }

  WithFoo.displayName = `withFoo(${WrappedComponent.displayName || WrappedComponent.name})`;
  return WithFoo;
}

@withFoo
class ChildComponent extends React.Component {
  render () {
    <div />
  }
}

const ParentComponent = () => <ChildComponent />

describe('ParentComponent', () => {
  it('renders ChildComponent (select by class)', function() {
    const wrapper = shallow(<ParentComponent />)

    expect(wrapper.is(ChildComponent)).to.be.true
  })

  it('renders ChildComponent (select by class name)', function() {
    const wrapper = shallow(<ParentComponent />)

    expect(wrapper.is('ChildComponent')).to.be.true
  })
})

The second test now fails!

It would need to be:

  it('renders ChildComponent (select by class name)', function() {
    const wrapper = shallow(<ParentComponent />)

    expect(wrapper.is('withFoo(ChildComponent)')).to.be.true
  })

I've been prefixing as this rule describes and in these cases I'll just import the component class and select by that expect(wrapper.is(ChildComponent)).to.be.true. However, this has always felt a little weird:

• Sometimes ChildComponent is not something that should be exported, it's private to ParentComponent. So I'd have to add an export specifically for this test.
• It forces you to import an additional component into the test file other than the component I'm directly testing.
• import ChildComponent from './ChildComponent'; ChildComponent.displayName !== 'ChildComponent' :neutral_face:

Nothing earth shattering, but something akin to code smells so I'm wondering how other people handle it.

[lencioni]

Related: https://github.com/airbnb/react-with-styles/issues/32#issuecomment-255425391

It is best to use wrapper.find() with a reference to the component instead of a string that describes the component (e.g. wrapper.find(MyComponent), not wrapper.find("MyComponent")). This will solve this problem for you when using any HOC, while also avoiding accidentally finding a different component than the one you expected that may have the same name.

For testing the underlying component with Enzyme, you probably want to use .dive().

I hope that helps! Let me know if you have any questions.

[jeffcarbs]

Cool, thanks for the response!