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

`injectEnvironment` called twice: `react-test-renderer` and `react-dom` #7386

Closed
NicolasT opened this Issue Jul 31, 2016 · 23 comments

Comments

Projects
None yet
@NicolasT

NicolasT commented Jul 31, 2016

Trying to test a very simple React component using Material-UI through Jest and react-test-renderer, I'm stuck because ReactCompositeComponent: injectEnvironment() can only be called once.

It looks like react-test-renderer calls injectEnvironment, but so does react-dom, which is (in my actual application) imported somewhere within a Material-UI module which is in turn imported by my component code.

This may not really be a 'bug' in React or react-test-renderer per se, though one could imagine other dependencies somehow loading react-dom as well, causing the same issue.

I created a test-case at https://github.com/NicolasT/react-test-renderer-and-react-dom-incompatible which may provide some more context.

react: 15.3.0
react-dom: 15.3.0
react-test-renderer: 15.3.0

@picodoth

This comment has been minimized.

Show comment
Hide comment
@picodoth

picodoth Aug 1, 2016

@NicolasT, I see the same issue. Seems like related to the change made in react @ 15.3.0, I downgrade react to 15.2.1 and the problem got solved. injectEnvironment() was called in 15.3.0 but not in 15.2.1.

picodoth commented Aug 1, 2016

@NicolasT, I see the same issue. Seems like related to the change made in react @ 15.3.0, I downgrade react to 15.2.1 and the problem got solved. injectEnvironment() was called in 15.3.0 but not in 15.2.1.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Aug 7, 2016

Member

This happens because the tool is assuming ReactDOM would be mocked out.
For example, with Jest, you could write:

jest.mock('react-dom');

to prevent this.

Ideally this shouldn’t be happening in the first place but there is more work to be done before we can avoid this.

Member

gaearon commented Aug 7, 2016

This happens because the tool is assuming ReactDOM would be mocked out.
For example, with Jest, you could write:

jest.mock('react-dom');

to prevent this.

Ideally this shouldn’t be happening in the first place but there is more work to be done before we can avoid this.

@ginter

This comment has been minimized.

Show comment
Hide comment
@ginter

ginter Aug 8, 2016

any other solutions for this? Mocking react-dom is giving me another error. I believe bc enzyme uses it.

ginter commented Aug 8, 2016

any other solutions for this? Mocking react-dom is giving me another error. I believe bc enzyme uses it.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Aug 8, 2016

Member

Maybe @cpojer could offer some insight. I think it should be possible to mock react-dom in one file but not the other, but it’s not clear to me from Jest docs what the “scope” of jest.mock is, and how to limit it to one file (or is it always limited?).

Member

gaearon commented Aug 8, 2016

Maybe @cpojer could offer some insight. I think it should be possible to mock react-dom in one file but not the other, but it’s not clear to me from Jest docs what the “scope” of jest.mock is, and how to limit it to one file (or is it always limited?).

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Aug 8, 2016

Member

I think it's always limited to one file.

Member

sophiebits commented Aug 8, 2016

I think it's always limited to one file.

@blainekasten

This comment has been minimized.

Show comment
Hide comment
@blainekasten

blainekasten Aug 9, 2016

Contributor

So I just looked into this a bit more. It seems that react-test-renderer and enzyme are not compatible due to this issue. In enzyme we do load react-dom and hook it up. Sure, jest could mock it, but other test frameworks might not be able to?

Additionally, i would assume people would want their snapshot tests next to their other tests. Currently that is not possible due to the above mentioned. Both renderers try to inject. Here's a type of test I would expect someone would want to write and couldn't at the moment.

import React from 'react';
import renderer from 'react-test-renderer';
import { mount } from 'enzyme';

class MyComponent extends React.Component {
  state = { active: true };

  render() {
    return (
      <div onClick={() => this.setState({active: !this.state.active}) }>
        Component is {this.state.active}
      </div>
   );
  }
}

describe('MyComponent', () => {
  it('matches snapshot', () => {
    const tree = renderer.create(<MyComponent />).toJSON();
    expect(tree).toMatchSnapshot();
  });

  it('updates state on click', () => {
    const wrapper = mount(<MyComponent />);
    wrapper.find('[onClick]').simulate('click');

    expect(wrapper).toHaveState('active', false);
  });
});

Is there anything I can do to help make this compatible? I'm just not sure what the solution you guys are looking for is.

Contributor

blainekasten commented Aug 9, 2016

So I just looked into this a bit more. It seems that react-test-renderer and enzyme are not compatible due to this issue. In enzyme we do load react-dom and hook it up. Sure, jest could mock it, but other test frameworks might not be able to?

Additionally, i would assume people would want their snapshot tests next to their other tests. Currently that is not possible due to the above mentioned. Both renderers try to inject. Here's a type of test I would expect someone would want to write and couldn't at the moment.

import React from 'react';
import renderer from 'react-test-renderer';
import { mount } from 'enzyme';

class MyComponent extends React.Component {
  state = { active: true };

  render() {
    return (
      <div onClick={() => this.setState({active: !this.state.active}) }>
        Component is {this.state.active}
      </div>
   );
  }
}

describe('MyComponent', () => {
  it('matches snapshot', () => {
    const tree = renderer.create(<MyComponent />).toJSON();
    expect(tree).toMatchSnapshot();
  });

  it('updates state on click', () => {
    const wrapper = mount(<MyComponent />);
    wrapper.find('[onClick]').simulate('click');

    expect(wrapper).toHaveState('active', false);
  });
});

Is there anything I can do to help make this compatible? I'm just not sure what the solution you guys are looking for is.

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Aug 9, 2016

Member

Yes, this is a limitation right now; you can't use ReactDOM and ReactTestRenderer in the same file unfortunately.

Member

sophiebits commented Aug 9, 2016

Yes, this is a limitation right now; you can't use ReactDOM and ReactTestRenderer in the same file unfortunately.

@aweary

This comment has been minimized.

Show comment
Hide comment
@aweary

aweary Aug 9, 2016

Member

Are there any plans to resolve that conflict or is it just something the community needs to be aware of? It seems like ReactTestRenderer would have to work with ReactComponentBrowserEnvironment or the injection invariant would have to be removed or skirted somehow (allowing environments to be replaced).

Member

aweary commented Aug 9, 2016

Are there any plans to resolve that conflict or is it just something the community needs to be aware of? It seems like ReactTestRenderer would have to work with ReactComponentBrowserEnvironment or the injection invariant would have to be removed or skirted somehow (allowing environments to be replaced).

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Aug 9, 2016

Member

I would like to change core modules to be instantiable instead of relying on a global injection, and then you could share two in the same environment. We might also switch to copying renderer files to react-test-renderer so that the modules are totally independent and don't have shared state.

Member

sophiebits commented Aug 9, 2016

I would like to change core modules to be instantiable instead of relying on a global injection, and then you could share two in the same environment. We might also switch to copying renderer files to react-test-renderer so that the modules are totally independent and don't have shared state.

@nwalters512

This comment has been minimized.

Show comment
Hide comment
@nwalters512

nwalters512 Aug 11, 2016

This bug also makes it difficult to write tests for components that use react-native-listener, which also relies on ReactDOM. Mocking out ReactDOM doesn't fix the problem and simply causes another error to be thrown.

nwalters512 commented Aug 11, 2016

This bug also makes it difficult to write tests for components that use react-native-listener, which also relies on ReactDOM. Mocking out ReactDOM doesn't fix the problem and simply causes another error to be thrown.

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Aug 13, 2016

Member

@spicyj The builds in master should support this since they copy so should we close this out?

Member

sebmarkbage commented Aug 13, 2016

@spicyj The builds in master should support this since they copy so should we close this out?

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Aug 13, 2016

Member

Sure. Doesn't look like your change is in 15.3.1 but when 15.4 is released, this should be fixed.

Member

sophiebits commented Aug 13, 2016

Sure. Doesn't look like your change is in 15.3.1 but when 15.4 is released, this should be fixed.

@gjchung

This comment has been minimized.

Show comment
Hide comment
@gjchung

gjchung Sep 12, 2016

When is 15.4 scheduled to be released?

gjchung commented Sep 12, 2016

When is 15.4 scheduled to be released?

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Sep 12, 2016

Member

I would expect it to ship within two weeks but that is my guess.

I'd like to see #7649 land before the release, and it looks like it will happen. It provides a way to work around another annoying issue with test renderer.

Then #7482 needs to be fixed because it's a regression and we'll break a lot of projects if we ship without fixing it. I know @zpao planned to look into it.

I think that when both are fixed/merged, 15.4.0 should be good to go.

Member

gaearon commented Sep 12, 2016

I would expect it to ship within two weeks but that is my guess.

I'd like to see #7649 land before the release, and it looks like it will happen. It provides a way to work around another annoying issue with test renderer.

Then #7482 needs to be fixed because it's a regression and we'll break a lot of projects if we ship without fixing it. I know @zpao planned to look into it.

I think that when both are fixed/merged, 15.4.0 should be good to go.

@gjchung

This comment has been minimized.

Show comment
Hide comment
@gjchung

gjchung Sep 12, 2016

Thanks for the update!

gjchung commented Sep 12, 2016

Thanks for the update!

@johnthepink johnthepink referenced this issue Sep 14, 2016

Merged

Jest Refactor #201

8 of 8 tasks complete

@fson fson referenced this issue Sep 26, 2016

Closed

Update Jest #757

mathewhawley pushed a commit to mathewhawley/react-redux-todo that referenced this issue Sep 30, 2016

@rande

This comment has been minimized.

Show comment
Hide comment
@rande

rande Oct 4, 2016

@gaearon look like the 2 issues from your last comment are now fixed. Do you plan a release ? This will help us a lot.

Thanks for your time.

rande commented Oct 4, 2016

@gaearon look like the 2 issues from your last comment are now fixed. Do you plan a release ? This will help us a lot.

Thanks for your time.

@aweary

This comment has been minimized.

Show comment
Hide comment
@aweary

aweary Oct 4, 2016

Member

@rande we have an open ticket for the next release #7770. A RC for 15.4.0 should be out soon based on #7840 (comment).

Member

aweary commented Oct 4, 2016

@rande we have an open ticket for the next release #7770. A RC for 15.4.0 should be out soon based on #7840 (comment).

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Oct 14, 2016

Member

You can try 15.4.0-rc.3. It should have this fix.

Member

gaearon commented Oct 14, 2016

You can try 15.4.0-rc.3. It should have this fix.

@rande

This comment has been minimized.

Show comment
Hide comment
@rande

rande Oct 18, 2016

@gaearon thanks, the jest's tests are now green, which is a good news ;)

The hot reload component does not seem to work anymore: Module not found: Error: Cannot resolve module 'react/lib/ReactMount', found this https://github.com/gaearon/react-hot-loader/blob/master/docs/README.md#usage-with-external-react

I will check your documentation.

rande commented Oct 18, 2016

@gaearon thanks, the jest's tests are now green, which is a good news ;)

The hot reload component does not seem to work anymore: Module not found: Error: Cannot resolve module 'react/lib/ReactMount', found this https://github.com/gaearon/react-hot-loader/blob/master/docs/README.md#usage-with-external-react

I will check your documentation.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Oct 18, 2016

Member

The hot reload component does not seem to work anymore

React Hot Loader 1.x has been deprecated and unsupported for about a year by now.

Member

gaearon commented Oct 18, 2016

The hot reload component does not seem to work anymore

React Hot Loader 1.x has been deprecated and unsupported for about a year by now.

@ruslansavenok

This comment has been minimized.

Show comment
Hide comment
@ruslansavenok

ruslansavenok Nov 6, 2016

doesn't work in 16.0.2.

Any workarounds before it will be fixed?

import React from 'react'
import ReactDOM from 'react-dom'
import InputSwitch from './main'
import TestUtils from 'react-addons-test-utils'
import renderer from 'react-test-renderer'

test('default value comes from checked prop', () => {
  const component = renderer.create(
    <InputSwitch checked={true} onChange={() => true} />
  )
  expect(component.toJSON()).toMatchSnapshot()
})


test('onChange prop is triggered with checkbox value', () => {
  let onChange = jest.fn()
  const component = TestUtils.renderIntoDocument(
    <InputSwitch checked={true} onChange={onChange} />
  )
  TestUtils.Simulate.change(
    TestUtils.findRenderedDOMComponentWithTag(component, 'input'),
    {
      target: {checked: false}
    }
  )
  expect(onChange).toBeCalledWith(false)
})
 Test suite failed to run

    Invariant Violation: ReactCompositeComponent: injectEnvironment() can only be called once.

      at invariant (node_modules/react/node_modules/fbjs/lib/invariant.js:38:15)
      at Object.injectEnvironment (node_modules/react/lib/ReactComponentEnvironment.js:36:60)
      at Object.<anonymous> (node_modules/react/lib/ReactTestRenderer.js:130:37)
      at Object.<anonymous> (node_modules/react-test-renderer/index.js:4:18)
      at Object.<anonymous> (app/es6/components/input_switch/input_switch.test.js:5:52)
      at process._tickCallback (internal/process/next_tick.js:103:7)

ruslansavenok commented Nov 6, 2016

doesn't work in 16.0.2.

Any workarounds before it will be fixed?

import React from 'react'
import ReactDOM from 'react-dom'
import InputSwitch from './main'
import TestUtils from 'react-addons-test-utils'
import renderer from 'react-test-renderer'

test('default value comes from checked prop', () => {
  const component = renderer.create(
    <InputSwitch checked={true} onChange={() => true} />
  )
  expect(component.toJSON()).toMatchSnapshot()
})


test('onChange prop is triggered with checkbox value', () => {
  let onChange = jest.fn()
  const component = TestUtils.renderIntoDocument(
    <InputSwitch checked={true} onChange={onChange} />
  )
  TestUtils.Simulate.change(
    TestUtils.findRenderedDOMComponentWithTag(component, 'input'),
    {
      target: {checked: false}
    }
  )
  expect(onChange).toBeCalledWith(false)
})
 Test suite failed to run

    Invariant Violation: ReactCompositeComponent: injectEnvironment() can only be called once.

      at invariant (node_modules/react/node_modules/fbjs/lib/invariant.js:38:15)
      at Object.injectEnvironment (node_modules/react/lib/ReactComponentEnvironment.js:36:60)
      at Object.<anonymous> (node_modules/react/lib/ReactTestRenderer.js:130:37)
      at Object.<anonymous> (node_modules/react-test-renderer/index.js:4:18)
      at Object.<anonymous> (app/es6/components/input_switch/input_switch.test.js:5:52)
      at process._tickCallback (internal/process/next_tick.js:103:7)
@cpojer

This comment has been minimized.

Show comment
Hide comment
@cpojer

cpojer Nov 6, 2016

Contributor

This will be resolved in React 15.4.

Contributor

cpojer commented Nov 6, 2016

This will be resolved in React 15.4.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Nov 16, 2016

Member

This was fixed in React 15.4.0 which is out today.

Member

gaearon commented Nov 16, 2016

This was fixed in React 15.4.0 which is out today.

@MoOx MoOx referenced this issue Nov 17, 2016

Closed

Upgrade to react@15.4.0 #255

1 of 4 tasks complete

gabro added a commit to buildo/react-components that referenced this issue Nov 17, 2016

gabro added a commit to buildo/react-components that referenced this issue Nov 17, 2016

gabro added a commit to buildo/react-components that referenced this issue Nov 17, 2016

gabro added a commit to buildo/react-components that referenced this issue Nov 17, 2016

@igolden igolden referenced this issue Apr 4, 2017

Closed

[New Docs] Wanted Guides #8060

1 of 13 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment