ReactTestRenderer doesn't work with refs or ReactDOM.findDOMNode #7371

Closed
ryanseddon opened this Issue Jul 29, 2016 · 18 comments

Comments

Projects
None yet
@ryanseddon
Contributor

ryanseddon commented Jul 29, 2016

Jest snapshot testing uses ReactTestRenderer but if my component contains a ref or uses ReactDOM.findDOMNode it throws TypeError: component.getPublicInstance is not a function.

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-test-renderer';

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

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao Jul 29, 2016

Member

Thanks for the well documented issue! I'll leave it to @spicyj since he did the hard work on this.

Member

zpao commented Jul 29, 2016

Thanks for the well documented issue! I'll leave it to @spicyj since he did the hard work on this.

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Jul 31, 2016

Member

Hm, after #7258 (which should be in 15.3.0) it give you null for any ref and should not throw. It's expected that ReactDOM.findDOMNode doesn't work.

Member

sophiebits commented Jul 31, 2016

Hm, after #7258 (which should be in 15.3.0) it give you null for any ref and should not throw. It's expected that ReactDOM.findDOMNode doesn't work.

@ryanseddon

This comment has been minimized.

Show comment
Hide comment
@ryanseddon

ryanseddon Aug 1, 2016

Contributor

So would it be safe to say that snapshot testing on something like react-frame-component wouldn't work?

Contributor

ryanseddon commented Aug 1, 2016

So would it be safe to say that snapshot testing on something like react-frame-component wouldn't work?

@zpao zpao added the Component: DOM label Aug 3, 2016

@KnutHelland

This comment has been minimized.

Show comment
Hide comment
@KnutHelland

KnutHelland Aug 5, 2016

Looks like React also logs a warning when using refs with react-test-renderer now:

Warning: Stateless function components cannot be given refs (See ref "main" in a component created by DummyComponent). Attempts to access this ref will fail.

caused by code similar to this example:

const DummyComponent = React.createClass({
    render() {
        return <div ref="main">content</div>;
    }
});

(I can put up an example on github if you want)

Looks like React also logs a warning when using refs with react-test-renderer now:

Warning: Stateless function components cannot be given refs (See ref "main" in a component created by DummyComponent). Attempts to access this ref will fail.

caused by code similar to this example:

const DummyComponent = React.createClass({
    render() {
        return <div ref="main">content</div>;
    }
});

(I can put up an example on github if you want)

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Aug 7, 2016

Member

So would it be safe to say that snapshot testing on something like react-frame-component wouldn't work?

It wouldn’t, but there are a few possible options:

  • You can mock it out (assuming you use Jest)
  • In the future test renderer could provide a first-class component mocking API out of the box, unrelated to Jest
  • We could give you a mock object that looks like a DOM node but doesn’t actually do anything

Any solutions you prefer?

Member

gaearon commented Aug 7, 2016

So would it be safe to say that snapshot testing on something like react-frame-component wouldn't work?

It wouldn’t, but there are a few possible options:

  • You can mock it out (assuming you use Jest)
  • In the future test renderer could provide a first-class component mocking API out of the box, unrelated to Jest
  • We could give you a mock object that looks like a DOM node but doesn’t actually do anything

Any solutions you prefer?

@travisbt

This comment has been minimized.

Show comment
Hide comment
@travisbt

travisbt Sep 1, 2016

We had the same problem, but solved it using the ref callback as described (with examples) here.
https://facebook.github.io/react/docs/more-about-refs.html#the-ref-callback-attribute

componentWillMount: function() {
  this._refs = {};
},
componentDidMount: function() {
  this._refs[`textRefId`].focus();
},
render: function() {
  return <TextInput ref={(c) => this._refs[`textRefId`] = c; } />;
},

travisbt commented Sep 1, 2016

We had the same problem, but solved it using the ref callback as described (with examples) here.
https://facebook.github.io/react/docs/more-about-refs.html#the-ref-callback-attribute

componentWillMount: function() {
  this._refs = {};
},
componentDidMount: function() {
  this._refs[`textRefId`].focus();
},
render: function() {
  return <TextInput ref={(c) => this._refs[`textRefId`] = c; } />;
},
@ryanseddon

This comment has been minimized.

Show comment
Hide comment
@ryanseddon

ryanseddon Sep 1, 2016

Contributor

@gaearon sorry missed your comment.

We could give you a mock object that looks like a DOM node but doesn’t actually do anything

That would be the preference as I'm asserting the children of the frame component.

Contributor

ryanseddon commented Sep 1, 2016

@gaearon sorry missed your comment.

We could give you a mock object that looks like a DOM node but doesn’t actually do anything

That would be the preference as I'm asserting the children of the frame component.

@aweary

This comment has been minimized.

Show comment
Hide comment
@aweary

aweary Sep 2, 2016

Member

I don't think providing a mock DOM node would be the best solution here. We'd have to implement the full DOM node API so that when it's actually used it doesn't throw. It would be better if we could have an optional integration with jsdom (or whatever the user chooses to use), so that we can use a full implementation of the DOM and avoid having to implement and maintain our own mock API.

Member

aweary commented Sep 2, 2016

I don't think providing a mock DOM node would be the best solution here. We'd have to implement the full DOM node API so that when it's actually used it doesn't throw. It would be better if we could have an optional integration with jsdom (or whatever the user chooses to use), so that we can use a full implementation of the DOM and avoid having to implement and maintain our own mock API.

@chase chase referenced this issue in facebook/create-react-app Oct 5, 2016

Merged

Upgrade to Jest 16 #858

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Nov 16, 2016

Member

Refs work with test renderer in master, and you can even mock them to return something else instead of null:

import React from 'react';

export default class MyInput extends React.Component {
  componentDidMount() {
    this.input.focus();
  }
  render() {
    return (
      <input ref={node => this.input = node} />
    );
  }
}
import React from 'react';
import MyInput from './MyInput';
import renderer from 'react-test-renderer';

function createNodeMock(element) {
  if (element.type === 'input') {
    return {
      focus() {},
    };
  }
  return null;
}

it('renders correctly', () => {
  const tree = renderer.create(
    <MyInput />,
    {createNodeMock}
  ).toJSON();
  expect(tree).toMatchSnapshot();
});

There are no plans to support findDOMNode() in test renderer because it should be agnostic of React DOM and there is no way to implement it in a way that won't break in the future.

Member

gaearon commented Nov 16, 2016

Refs work with test renderer in master, and you can even mock them to return something else instead of null:

import React from 'react';

export default class MyInput extends React.Component {
  componentDidMount() {
    this.input.focus();
  }
  render() {
    return (
      <input ref={node => this.input = node} />
    );
  }
}
import React from 'react';
import MyInput from './MyInput';
import renderer from 'react-test-renderer';

function createNodeMock(element) {
  if (element.type === 'input') {
    return {
      focus() {},
    };
  }
  return null;
}

it('renders correctly', () => {
  const tree = renderer.create(
    <MyInput />,
    {createNodeMock}
  ).toJSON();
  expect(tree).toMatchSnapshot();
});

There are no plans to support findDOMNode() in test renderer because it should be agnostic of React DOM and there is no way to implement it in a way that won't break in the future.

@romanoff

This comment has been minimized.

Show comment
Hide comment
@romanoff

romanoff Nov 28, 2016

For some reason I still can't use ref in jest tests. Details here: http://stackoverflow.com/questions/40852131/cant-get-ref-in-jest-tests

For some reason I still can't use ref in jest tests. Details here: http://stackoverflow.com/questions/40852131/cant-get-ref-in-jest-tests

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Nov 28, 2016

Member

@romanoff

React test renderer is not coupled to React DOM. It can't "guess" which DOM APIs your component relies on. You need to mock the nodes yourself, as noted in 15.4.0 release notes. I hope this helps!

Member

gaearon commented Nov 28, 2016

@romanoff

React test renderer is not coupled to React DOM. It can't "guess" which DOM APIs your component relies on. You need to mock the nodes yourself, as noted in 15.4.0 release notes. I hope this helps!

@arbesfeld arbesfeld referenced this issue in bvaughn/react-virtualized Dec 5, 2016

Closed

Usage with react-test-renderer #493

wincent added a commit to wincent/graphiql that referenced this issue Dec 9, 2016

Upgrade dependencies to fix build breakages
We've had some lint-related build failures:

  https://travis-ci.org/graphql/graphiql/builds/181453346

lately. Unfortunately, these masked test failures introduced by
35d8d38. When we fixed the lint (1eeb36c), the test failures showed
up:

  https://travis-ci.org/graphql/graphiql/builds/182431531

This commit fixes that by upgrading our deps:

- Replace "react-addons-test-utils", which access a path that doesn't
  exist in current "react-dom", with "react-test-renderer".

This required some API updates in the tests, but also some changes to
use ref-based DOM node access rather than `ReactDOM.findDOMNode`, which
doesn't work in "react-test-renderer".

See:

- facebook/react#7371
- facebook/react#8324

> Yea sorry, refs can work but `findDOMNode()` can't (we tried).

@wincent wincent referenced this issue in graphql/graphiql Dec 9, 2016

Merged

Upgrade dependencies to fix build breakages #238

leebyron added a commit to graphql/graphiql that referenced this issue Dec 9, 2016

Upgrade dependencies to fix build breakages (#238)
* Upgrade dependencies to fix build breakages

We've had some lint-related build failures:

  https://travis-ci.org/graphql/graphiql/builds/181453346

lately. Unfortunately, these masked test failures introduced by
35d8d38. When we fixed the lint (1eeb36c), the test failures showed
up:

  https://travis-ci.org/graphql/graphiql/builds/182431531

This commit fixes that by upgrading our deps:

- Replace "react-addons-test-utils", which access a path that doesn't
  exist in current "react-dom", with "react-test-renderer".

This required some API updates in the tests, but also some changes to
use ref-based DOM node access rather than `ReactDOM.findDOMNode`, which
doesn't work in "react-test-renderer".

See:

- facebook/react#7371
- facebook/react#8324

> Yea sorry, refs can work but `findDOMNode()` can't (we tried).

* Update package.json

@VeeteshJain VeeteshJain referenced this issue in CezaryDanielNowak/React-dotdotdot Dec 22, 2016

Closed

TypeError: Cannot read property 'length' of null #14

@ericgio ericgio referenced this issue in ericgio/react-bootstrap-typeahead Feb 15, 2017

Closed

Fail snapshot test after including the typeahead #139

@justin808 justin808 referenced this issue in YouCanBookMe/react-datetime Feb 19, 2017

Closed

Cannot test with storyshots due to findDomNode #275

@mweststrate

This comment has been minimized.

Show comment
Hide comment
@mweststrate

mweststrate Mar 10, 2017

What is the recommended heuristic to detect that findDOMNode won't work, in order to take a different code path? (try / catch could work but isn't very clear)

Edit: to elaborate: this is for a component library, so mocking (inside the library at least) is not an option

mweststrate commented Mar 10, 2017

What is the recommended heuristic to detect that findDOMNode won't work, in order to take a different code path? (try / catch could work but isn't very clear)

Edit: to elaborate: this is for a component library, so mocking (inside the library at least) is not an option

@hyzhak

This comment has been minimized.

Show comment
Hide comment
@hyzhak

hyzhak May 31, 2017

Is there any idea how to overcome this bug? I have few third party components (like https://github.com/ericgio/react-bootstrap-typeahead/) which uses ReactDOM.findDOMNode how could I use jest snapshots with them?

hyzhak commented May 31, 2017

Is there any idea how to overcome this bug? I have few third party components (like https://github.com/ericgio/react-bootstrap-typeahead/) which uses ReactDOM.findDOMNode how could I use jest snapshots with them?

@a7madgamal

This comment has been minimized.

Show comment
Hide comment
@a7madgamal

a7madgamal Jul 24, 2017

any help for the 10-thumbs-up comment will be really appreciated :D

any help for the 10-thumbs-up comment will be really appreciated :D

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jul 24, 2017

Member

This is not a bug. As explained above it is intentional.

The workaround is simple if you use jest. Just mock the third party component causing issues.

For example:

jest.mock('third-party-button', () => 'ThirdPartyButton');

Put this at the top of your test file.

Now any imports of third-party-button (replace this with your component) will become a string (e.g. ThirdPartyButton) so the component will become a “leaf” in the snapshot, like a div. Of course this won't actually test it, but it makes sense to only test your own code.

For libraries exporting multiple components, the workaround is similar but you'd want to provide a different mock. Something like:

jest.mock('third-party-lib', () => {
  return {
    ThirdPartyButton: 'ThirdPartyButton',
    OtherThing: 'OtherThing',
  };
}));

Finally, as last option, you can mock react-dom itself.

jest.mock('react-dom', () => ({
  findDOMNode: () => ({}),
});

Note you can return anything there and adjust it to match the expectations of the tested code.

All of this works if you use Jest. Unfortunately I don’t have good solutions for other runners.

Member

gaearon commented Jul 24, 2017

This is not a bug. As explained above it is intentional.

The workaround is simple if you use jest. Just mock the third party component causing issues.

For example:

jest.mock('third-party-button', () => 'ThirdPartyButton');

Put this at the top of your test file.

Now any imports of third-party-button (replace this with your component) will become a string (e.g. ThirdPartyButton) so the component will become a “leaf” in the snapshot, like a div. Of course this won't actually test it, but it makes sense to only test your own code.

For libraries exporting multiple components, the workaround is similar but you'd want to provide a different mock. Something like:

jest.mock('third-party-lib', () => {
  return {
    ThirdPartyButton: 'ThirdPartyButton',
    OtherThing: 'OtherThing',
  };
}));

Finally, as last option, you can mock react-dom itself.

jest.mock('react-dom', () => ({
  findDOMNode: () => ({}),
});

Note you can return anything there and adjust it to match the expectations of the tested code.

All of this works if you use Jest. Unfortunately I don’t have good solutions for other runners.

@a7madgamal

This comment has been minimized.

Show comment
Hide comment
@a7madgamal

a7madgamal Jul 24, 2017

Thanks a lot for this helpful comment 👍🏻
I'm sure your explanation will be helpful to many jest noobz like myself!

Thanks a lot for this helpful comment 👍🏻
I'm sure your explanation will be helpful to many jest noobz like myself!

@ktj

This comment has been minimized.

Show comment
Hide comment
@ktj

ktj Jul 31, 2017

This is how I mocked 3rd party dependency that used react-dom findDOMNode in my app.

/// SomeComponent.test.js
jest.mock('some-library/lib/MockThis', () => {
  const Mock = require.requireActual('./MockForMockThis');
  return Mock;
});

// test....

Drawback in this is that if that 3rd party dependency changes diretory structure etc, then this will break.

ktj commented Jul 31, 2017

This is how I mocked 3rd party dependency that used react-dom findDOMNode in my app.

/// SomeComponent.test.js
jest.mock('some-library/lib/MockThis', () => {
  const Mock = require.requireActual('./MockForMockThis');
  return Mock;
});

// test....

Drawback in this is that if that 3rd party dependency changes diretory structure etc, then this will break.

@tirli tirli referenced this issue in storybooks/storybook Aug 2, 2017

Closed

Storyshots. Ability to skip some stories #1578

@joaovieira joaovieira referenced this issue in danreeves/react-tether Aug 14, 2017

Closed

Replace findDOMNode with ref. #50

@gurre

This comment has been minimized.

Show comment
Hide comment
@gurre

gurre Aug 19, 2017

Mocking refs and canvas was easy using:

jest.mock('react-dom', () => ({
    findDOMNode: () => ({
      getContext: jest.fn(),
    }),
  })
);

gurre commented Aug 19, 2017

Mocking refs and canvas was easy using:

jest.mock('react-dom', () => ({
    findDOMNode: () => ({
      getContext: jest.fn(),
    }),
  })
);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment