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

Proposals for the ReactTestRenderer API #7148

Closed
josephsavona opened this issue Jun 29, 2016 · 21 comments
Closed

Proposals for the ReactTestRenderer API #7148

josephsavona opened this issue Jun 29, 2016 · 21 comments

Comments

@josephsavona
Copy link
Contributor

josephsavona commented Jun 29, 2016

Do you want to request a feature or report a bug?
New feature

What is the current behavior?
ReactTestRenderer returns an "instance", but this primarily seems to support two things: getting a representation of the "rendered" output and calling methods on the instance (via instance.getInstance().componentMethod()).

What is the expected behavior?
This is good for testing regular UI components, but testing infra-level components (HOCs) often requires testing lifecycle hooks. I've found that I need the ability to re-render the component with new props (test componentWillReceiveProps, shouldComponentUpdate, etc) and unmount the component altogether (test componentWillUnmount).

Changing props can be accomplished via a helper (I'm using the following to work around this issue):

class PropsSetter extends React.Component {
    constructor() {
      super();
      this.state = {
        active: true,
        props: null,
      };
    }
    setProps(props) {
      this.setState({props});
    }
    unmount() {
      this.setState({active: false});
    }
    render() {
      if (!this.state.active) {
         return <div />;
      }
      const child = React.Children.only(this.props.children);
      if (this.state.props) {
        return React.cloneElement(child, this.state.props);
      }
      return child;
    }
  }

const inst = ReactTestRenderer.create(
  <PropsSetter>
    <ComponentToTest ... />
  </PropsSetter>
);
inst.getInstance().setProps({...});

This works for updating props, but calling PropsSetter#unmount() fails with ReactComponentEnvironment.replaceNodeWithMarkup is not a function (appears that replacing the component with the dummy <div /> is failing).

Preferably the API would support both of these directly:

const inst = ReactTestRenderer.create(<ComponentToTest ... />);
inst.render(props); // updates with new props
inst.ummount(); // unmounts component

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

This is a new API only on master (to my knowledge).

@sophiebits
Copy link
Contributor

This works for updating props, but calling PropsSetter#unmount() fails with ReactComponentEnvironment.replaceNodeWithMarkup is not a function (appears that replacing the component with the dummy <div /> is failing).

As a temporary workaround, alternating between <div /> and <div>{this.props.children}</div> (essentially, adding an extra wrapping div) might fix this error.

@sophiebits
Copy link
Contributor

Going to use this issue to track other bugs/improvements:

@cpojer said that when replacing the Image.js mock in facebook/react-native@7c53add with module.exports = 'Image';, this error was thrown:

TypeError: component.getPublicInstance is not a function on ReactCompositeComponentMixin.attachRef (node_modules/react/lib/ReactCompositeComponent.js:823:45)

Pretty sure that should work (and if not, give a better error message).

@josephsavona
Copy link
Contributor Author

As a temporary workaround, alternating between

and
{this.props.children}
...

No luck, got this error instead: ReactComponentEnvironment.processChildrenUpdates is not a function. I'm working around this for now with instance.getInstance().componentWillUnmount() ;-)

@sebmarkbage
Copy link
Collaborator

sebmarkbage commented Jul 6, 2016

It would be nice to be able to traverse the tree.

TestComponent::getChildren() => Array<TestComponent>

TestComponent::getType() : (typeof ReactComponent) | ReactStatelessFunction | string

TestComponent::getInstance() : ReactComponent | null

TestComponent::getProps() : Object

TestComponent::toJSON() : Same as before

sophiebits added a commit to sophiebits/react that referenced this issue Jul 13, 2016
Adds `.update(newElement)` and `.unmount()` and makes children reorders and composite type swapping work.

Part of facebook#7148.
sophiebits added a commit that referenced this issue Jul 13, 2016
Adds `.update(newElement)` and `.unmount()` and makes children reorders and composite type swapping work.

Part of #7148.
zpao pushed a commit that referenced this issue Jul 13, 2016
Adds `.update(newElement)` and `.unmount()` and makes children reorders and composite type swapping work.

Part of #7148.
(cherry picked from commit caec8d5)
@ryanseddon
Copy link
Contributor

Currently trying to use this with jest snapshot testing but if a component contains a ref or makes a call to ReactDOM.findDOMNode it fails.

Component

import React from 'react';

export default class Link extends React.Component {
  render() {
    return (
      <a
        ref={a => this._a = a}
        href={this.props.page || '#'}>
        {this.props.children}
      </a>
    );
  }
}

Test

'use strict'

import React from 'react';
import Link from '../Link';
import renderer from 'react/lib/ReactTestRenderer';

describe('Link', () => {
  it('renders correctly', () => {
    const tree = renderer.create(
      <Link page="foo" />
    ).toJSON();

    expect(tree).toMatchSnapshot();
  });
});

stack trace

 FAIL  __tests__/Link-test.js (2.148s)
● Link › it renders correctly
  - TypeError: component.getPublicInstance is not a function
        at attachRef (node_modules/react/lib/ReactRef.js:20:19)
        at Object.ReactRef.attachRefs (node_modules/react/lib/ReactRef.js:42:5)
        at attachRefs (node_modules/react/lib/ReactReconciler.js:26:12)
        at CallbackQueue._assign.notifyAll (node_modules/react/lib/CallbackQueue.js:67:22)
        at ReactTestReconcileTransaction.ON_DOM_READY_QUEUEING.close (node_modules/react/lib/ReactTestReconcileTransaction.js:37:26)
        at ReactTestReconcileTransaction.Mixin.closeAll (node_modules/react/lib/Transaction.js:204:25)
        at ReactTestReconcileTransaction.Mixin.perform (node_modules/react/lib/Transaction.js:151:16)
        at batchedMountComponentIntoNode (node_modules/react/lib/ReactTestMount.js:61:27)
        at ReactDefaultBatchingStrategyTransaction.Mixin.perform (node_modules/react/lib/Transaction.js:138:20)
        at Object.ReactDefaultBatchingStrategy.batchedUpdates (node_modules/react/lib/ReactDefaultBatchingStrategy.js:63:19)

@zpao
Copy link
Member

zpao commented Jul 29, 2016

@ryanseddon Can you file a new issue for your specific problem, this issue is about tracking other APIs.

@ryanseddon
Copy link
Contributor

apologies, done #7371

@cpojer
Copy link
Contributor

cpojer commented Sep 14, 2016

Did you guys think about enabling shallow rendering support in the test renderer? See jestjs/jest#1683

@sophiebits
Copy link
Contributor

Maybe you can just make the pretty-printer understand the output of render (that is, a React element). I don't know what else we might add that would be helpful.

@cpojer
Copy link
Contributor

cpojer commented Sep 15, 2016

Ah, that's an interesting idea. We could allow people to specify how deep react components should render? Would you prefer that over a shallow render API for the test renderer?

@sophiebits
Copy link
Contributor

I don't think I understand what you want. The test renderer does deep renders and gives you a JSON blob you can inspect. The shallow renderer API does shallow renders and gives you a React element tree that you can inspect. Do you want something different?

@cpojer
Copy link
Contributor

cpojer commented Sep 15, 2016

My understanding is that shallow rendering only renders one level deep and it is a separate renderer as well. I was wondering if we could use the test renderer to do the same or to add an API that only returns the shallow rendered tree (only 1 level deep), if that makes sense. I'm sorry if I'm missing something here.

@sophiebits
Copy link
Contributor

What would be the advantage over the existing shallow renderer?

@cpojer
Copy link
Contributor

cpojer commented Sep 15, 2016

I'm not sure I'm up to date on how it works. Which render target does it use? Can I use the shallow renderer with react-test-renderer?

@sophiebits
Copy link
Contributor

Here is the usage example in the docs:

https://facebook.github.io/react/docs/test-utils.html#shallow-rendering

From the technical implementation I don't think I would describe it exactly as a separate renderer, but it doesn't rely on either react-dom or react-test-renderer. You should be able to use it at the same time as either.

@cpojer
Copy link
Contributor

cpojer commented Sep 15, 2016

Since we already have a pretty printer for react elements for the diff engine it seems like snapshotting a shallow render output should just work then, right?

@sophiebits
Copy link
Contributor

The return value is in a slightly different format (React elements instead of JSON), which is what I meant above by adding support for pretty-printing React elements.

@cpojer
Copy link
Contributor

cpojer commented Sep 15, 2016

Got it, yeah we already do that in pretty-format. @gaearon snuck it in.

@khankuan
Copy link

khankuan commented Nov 9, 2016

Hi, I'm trying to use snapshot + shallow:

const shallowRenderer = ReactTestUtils.createRenderer();
const result = shallowRenderer.render(<Component {...props} />);
expect(result).toMatchSnapshot();

It works pretty cool :D

@faceyspacey
Copy link

faceyspacey commented Jan 6, 2017

@cpojer I think the ability to specify an integer to determine render depth makes a whole lot of sense.

Currently if you're using just jest you're left doing a lot of work mocking components to achieve the shallow render you get for free with react-addons-test-utils. A developer is willing to absorb this cost as a tradeoff for deep rendering.

However having to make the tradeoff could be avoided with:

const number = 3
const testInstance = renderer.create(<Component {...props} />, number)

I think it's a nice fit for jest. With something as powerful as Jest, using react-addons-test-utils directly shouldn't be happening anymore. One reason is because getRenderOutput() formats its output differently than toJSON, namely .children is always accessed from props, i.e. props.children.

@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

We have traversal now.

https://reactjs.org/docs/test-renderer.html

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

No branches or pull requests

9 participants