Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better test facilities for stateless components #4972

Closed
jfairbank opened this issue Sep 25, 2015 · 14 comments
Closed

Better test facilities for stateless components #4972

jfairbank opened this issue Sep 25, 2015 · 14 comments

Comments

@jfairbank
Copy link
Contributor

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.

@blainekasten
Copy link
Contributor

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

@ngerritsen
Copy link

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?

@ngerritsen
Copy link

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.

@jquense
Copy link
Contributor

jquense commented Oct 15, 2015

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

@iamdustan
Copy link
Contributor

Thanks for posting @jquense. Really helpful.

@alexprice1
Copy link

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...

@oliverwoodings
Copy link

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

@alexprice1
Copy link

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

@alexprice1
Copy link

@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.

@oliverwoodings
Copy link

@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.

@afc163
Copy link

afc163 commented Nov 30, 2015

@ngerritsen 👍 Realy helpful

@lauterry
Copy link

lauterry commented Jun 8, 2016

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 ?

@ngerritsen
Copy link

@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.

@gaearon
Copy link
Collaborator

gaearon commented Jan 6, 2018

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

@gaearon gaearon closed this as completed Jan 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants