facebook/react

Better test facilities for stateless components

jfairbank opened this issue ยท 14 comments

I originally left a comment in #4936, but I felt this might be better as a separate issue.

I completely agree with the rationale behind no backing instances and no refs for stateless components. I want to use pure stateless components as much as possible.

However, I'm running into instances where testing stateless components gets tricky. For the most part, using the shallow renderer to render a stateless component has been fine. I can then make assertions on expected props and children. To reduce the duplication, I abstract out the shallow renderer process to a helper function.

When, I want to test events, though, I have to take a different route. If my component takes an onClick prop, then I wrap it in a class component, so I can access the DOM node to simulate my click event. Again, this isn't a huge deal because I can abstract out the wrap/render process into a helper function.

I bring all this up because I feel that it would be nice to have better facilities in TestUtils for stateless components. I don't think the answer is allowing ReactDOM.findDOMNode to work on stateless components because that opens up it to abuse in normal application code. Are you open to allowing extra functions in TestUtils to simplify the process of testing stateless components? Or am I creating a code smell in my testing?

I'd be happy to create a PR for TestUtils if you think some more helper functions for stateless components would be beneficial.

What if TestUtils.Simulate.* checked for a backing instance, if it's not available it created a backing instance around the stateless component and handled it. I feel like this could all be handled internally. If the team is cool with this I'd write up some code for it.

cc @sebmarkbage

I ran in to the same issue. After happily converting some of my components to the stateless shorthand version (which is by the way, awesome), my tests did not succeed anymore because the find/srcyRenderedDOMComponent(s).x methods did not work anymore. Any suggestions on how to test these?

I actually made a component that wraps a stateless component into a regular component (without any wrapper html to be added, the output should be the same)

https://github.com/ngerritsen/react-stateless-wrapper

With this you can just use the normal testing facilities.

for the stake of posterity I'd like to post what I had to do to get our little testing framework to work nicely with Stateless components: https://github.com/jquense/teaspoon/blob/master/src/utils.js#L26

Its not pretty, I would love some more first class hooks into instance tree traversal and related so we can build better test tooling without needing to dig so deeply into react's private apis

Thanks for posting @jquense. Really helpful.

Yeah, I would love if we could add refs to stateless components, and it only worked for testing. Is this feasible at all?

With stateless components, my javascript is beautiful, but my tests are really ugly...

The work I have been doing on react-test-tree might be of interest here. Stateless components hit us hard too, since the original premise of test-tree was that it built a walkable tree out of refs. Stateless components obviously mess that up! The discussion we have been having is here: https://github.com/QubitProducts/react-test-tree/issues/49

The impression I have got from what @jimfb and @sebmarkbage have been saying is that refs really only serve a very specific purpose, and right now many things abuse them. In reality, they should be used purely for when React's API does not yet provide an official alternative. If you look at the React TestUtils, nowhere do they even talk or mention using refs for testing. It seems like in reality refs have just became an unintentional convenience for testing.

Given this, we have now abandoned using refs in react-test-tree and are implementing a new system using custom props. It looks a bit like this:

var MyComponent = React.createClass({
  render: function () {
    return (
      <div testRef="foo">
        <ul testRefCollection="bar">
          <li />
          <li>foobar</li>
        </ul>
        <MyComponent2 testRef="baz" />
      </div>
    );
  }
});

var MyComponent2 = React.createClass({
  render: function () {
    return (
      <div>
        <button testRef="boz" />
      </div>
    );
  }
});

var tree = testTree(<MyComponent />);
tree.get("foo").click();
tree.get("bar").length === 2;
tree.get("bar")[1].innerText === "foobar";
tree.getIn(["baz", "boz"]).click();

I believe this creates a much clearer distinction between regular refs and testing refs. We are also hoping to write a babel transform which can strip these testing props out in production. There is a working version out on the react0.14 branch if anyone wants to try it out: https://github.com/QubitProducts/react-test-tree/tree/react0.14

Awesome! This is a really good idea. I will talk to my coworkers about this.

@oliverwoodings we would love if react officially supported something similar to what your team has created, that way we have more assurance on the longevity of the support of a tool like this.

@chapinka yeah I can understand that concern. I would love for something like this to end up in a react, but I can't see it happening that quickly.

I can assure you that I will be maintaining test-tree for as long as possible. We have many internal projects that use it. Also it doesn't affect production code, so if for some reason test-tree did stop being updated you would only need to adjust your tests.

@ngerritsen ๐Ÿ‘ Realy helpful

It is acceptable to test functional component as follow :

it('should display Highlights', () => {
        var props = {
            highlights: [
                {"label": "HighlightItem 1", "picto": "picto1.svg"},
                {"label": "HighlightItem 2", "picto": "picto2.svg"},
                {"label": "HighlightItem 3", "picto": "picto3.svg"}
            ]
        };

        var highlights = Highlights(props);

        expect(highlights.props.children.length).toEqual(3);
        expect(highlights.props.children[0].type.name).toEqual("HighlightItem");
        expect(highlights.props.children[1].type.name).toEqual("HighlightItem");
        expect(highlights.props.children[2].type.name).toEqual("HighlightItem");
    });

In fact, I treat them as a function when testing.

What do you think ?

@lauterry What is acceptable for you project is up to you, put I think it's a pretty decent way to do it. However a good way to render could be to use Enzyme for shallow rendering.

Seems like most people are pretty happy with Enzyme. I think we can close this.

If you have specific API proposals for React itself please create an RFC: https://github.com/reactjs/rfcs