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

Need a test for setState in shallow render #4019

Closed
glenjamin opened this issue Jun 3, 2015 · 15 comments
Closed

Need a test for setState in shallow render #4019

glenjamin opened this issue Jun 3, 2015 · 15 comments

Comments

@glenjamin
Copy link
Contributor

Related to #2393

Rendering a component like this:

<div onClick={() => this.setState({something: 1}) />

rendering via shallow render, then calling handler

var result = shallowRenderer.getRenderOutput();
result.props.onClick()

Produces the following:

    ReferenceError: document is not defined
      at getActiveElement (myapp/node_modules/react/lib/getActiveElement.js:23:12)
      at ReactReconcileTransaction.ReactInputSelection.getSelectionInformation (myapp/node_modules/react/lib/ReactInputSelection.js:40:23)
      at ReactReconcileTransaction.Mixin.initializeAll (myapp/node_modules/react/lib/Transaction.js:168:30)
      at ReactReconcileTransaction.Mixin.perform (myapp/node_modules/react/lib/Transaction.js:133:12)
      at ReactUpdatesFlushTransaction.Mixin.perform (myapp/node_modules/react/lib/Transaction.js:134:20)
      at ReactUpdatesFlushTransaction.assign.perform (myapp/node_modules/react/lib/ReactUpdates.js:95:38)
      at Object.flushBatchedUpdates (myapp/node_modules/react/lib/ReactUpdates.js:175:19)
      at Object.wrapper [as flushBatchedUpdates] (myapp/node_modules/react/lib/ReactPerf.js:70:21)
      at ReactDefaultBatchingStrategyTransaction.Mixin.closeAll (myapp/node_modules/react/lib/Transaction.js:207:25)
      at ReactDefaultBatchingStrategyTransaction.Mixin.perform (myapp/node_modules/react/lib/Transaction.js:148:16)
      at Object.ReactDefaultBatchingStrategy.batchedUpdates (myapp/node_modules/react/lib/ReactDefaultBatchingStrategy.js:66:19)
      at Object.enqueueUpdate (myapp/node_modules/react/lib/ReactUpdates.js:215:22)
      at enqueueUpdate (myapp/node_modules/react/lib/ReactUpdateQueue.js:30:18)
      at Object.ReactUpdateQueue.enqueueSetState (myapp/node_modules/react/lib/ReactUpdateQueue.js:208:5)
      at [object Object].ReactComponent.setState (myapp/node_modules/react/lib/ReactComponent.js:69:20)
      at Object.React.createElement.React.createElement.React.createElement.onChange (myapp/client/components/LoginForm.js:30:35)

Ideally, this module should realise it's running on the server, and skip this behaviour check

@jimfb
Copy link
Contributor

jimfb commented Jun 5, 2015

cc @graue

chantastic added a commit to chantastic/react-media-object that referenced this issue Aug 9, 2015
This curcumvents a bug ir React. You can see a manifestation of it [here](facebook/react#4019).

This existence of a `document` is required by React, when using `setState`. While `shallowRenderer` is inteded for use without a document, all tests that use `setState` will fail with a `documentis not defined`reference error.

Fortunately, React doesn't need anything more than the existance of the global. So, we can define it in our test environment.

This is where a more robust environment could be setup, using [jsdom](https://www.npmjs.com/package/jsdom) if needed.
@gaearon
Copy link
Collaborator

gaearon commented Aug 26, 2015

I think I don't have this problem in 0.14, can somebody else verify?

@sophiebits
Copy link
Collaborator

40b7c19 may have fixed this. Not sure it actually rerenders though?

@glenjamin
Copy link
Contributor Author

Just tried this on 0.14.0-beta3 - reproduces in the same way as 0.13.

> node -e 'console.log(process.versions)'
{ http_parser: '2.3',
  node: '0.12.7',
  v8: '3.28.71.19',
  uv: '1.6.1',
  zlib: '1.2.8',
  modules: '14',
  openssl: '1.0.1p' }
var React = require('react');
var TestUtils;
if (React.version.substring(4) == '0.13') {
  TestUtils = require('react/addons').addons.TestUtils;
} else {
  TestUtils = require('react-addons-test-utils');
}

if (process.env.document) {
  global.document = {};
}

var renderer = TestUtils.createRenderer();

var Node = React.createClass({
  getInitialState: function() {
    return { foo: 'bar' };
  },
  clicked: function() {
    this.setState({foo: 'baz'});
  },
  render: function() {
    return (
      React.createElement('div', {
        onClick: this.clicked
      }, this.state.foo)
    );
  }
});

renderer.render(
  React.createElement(Node)
);

renderer.getRenderOutput().props.onClick();

console.log(renderer.getRenderOutput().props.children);

0.13

> node sample.js
/myapp/node_modules/react/lib/getActiveElement.js:23
    return document.body;
           ^
ReferenceError: document is not defined
    at getActiveElement (/myapp/node_modules/react/lib/getActiveElement.js:23:12)
    at ReactReconcileTransaction.ReactInputSelection.getSelectionInformation (/myapp/node_modules/react/lib/ReactInputSelection.js:40:23)
    at ReactReconcileTransaction.Mixin.initializeAll (/myapp/node_modules/react/lib/Transaction.js:168:30)
    at ReactReconcileTransaction.Mixin.perform (/myapp/node_modules/react/lib/Transaction.js:133:12)
    at ReactUpdatesFlushTransaction.Mixin.perform (/myapp/node_modules/react/lib/Transaction.js:134:20)
    at ReactUpdatesFlushTransaction.assign.perform (/myapp/node_modules/react/lib/ReactUpdates.js:95:38)
    at Object.flushBatchedUpdates (/myapp/node_modules/react/lib/ReactUpdates.js:175:19)
    at Object.wrapper [as flushBatchedUpdates] (/myapp/node_modules/react/lib/ReactPerf.js:70:21)
    at ReactDefaultBatchingStrategyTransaction.Mixin.closeAll (/myapp/node_modules/react/lib/Transaction.js:207:25)
    at ReactDefaultBatchingStrategyTransaction.Mixin.perform (/myapp/node_modules/react/lib/Transaction.js:148:16)

0.13 with document hack

> document=1 node sample.js
baz

0.14.0-beta4

> node sample.js
/myapp/node_modules/react/node_modules/fbjs/lib/getActiveElement.js:25
    return document.body;
           ^
ReferenceError: document is not defined
    at getActiveElement (/myapp/node_modules/react/node_modules/fbjs/lib/getActiveElement.js:25:12)
    at ReactReconcileTransaction.ReactInputSelection.getSelectionInformation (/myapp/node_modules/react/lib/ReactInputSelection.js:38:23)
    at ReactReconcileTransaction.Mixin.initializeAll (/myapp/node_modules/react/lib/Transaction.js:168:75)
    at ReactReconcileTransaction.Mixin.perform (/myapp/node_modules/react/lib/Transaction.js:135:12)
    at ReactUpdatesFlushTransaction.Mixin.perform (/myapp/node_modules/react/lib/Transaction.js:136:20)
    at ReactUpdatesFlushTransaction.assign.perform (/myapp/node_modules/react/lib/ReactUpdates.js:86:38)
    at Object.flushBatchedUpdates (/myapp/node_modules/react/lib/ReactUpdates.js:147:19)
    at Object.wrapper [as flushBatchedUpdates] (/myapp/node_modules/react/lib/ReactPerf.js:66:21)
    at ReactDefaultBatchingStrategyTransaction.Mixin.closeAll (/myapp/node_modules/react/lib/Transaction.js:202:25)
    at ReactDefaultBatchingStrategyTransaction.Mixin.perform (/myapp/node_modules/react/lib/Transaction.js:149:16)

0.14.0-beta3 with document hack

> document=1 node sample.js
baz

There's no git tags for the 0.14 series, but from a grep around it looks like that commit is present in beta3.

chantastic added a commit to chantastic/react-media-object that referenced this issue Sep 2, 2015
This curcumvents a bug ir React. You can see a manifestation of it [here](facebook/react#4019).

This existence of a `document` is required by React, when using `setState`. While `shallowRenderer` is inteded for use without a document, all tests that use `setState` will fail with a `documentis not defined`reference error.

Fortunately, React doesn't need anything more than the existance of the global. So, we can define it in our test environment.

This is where a more robust environment could be setup, using [jsdom](https://www.npmjs.com/package/jsdom) if needed.
chantastic added a commit to chantastic/react-media-object that referenced this issue Sep 2, 2015
This curcumvents a bug ir React. You can see a manifestation of it [here](facebook/react#4019).

This existence of a `document` is required by React, when using `setState`. While `shallowRenderer` is inteded for use without a document, all tests that use `setState` will fail with a `documentis not defined`reference error.

Fortunately, React doesn't need anything more than the existance of the global. So, we can define it in our test environment.

This is where a more robust environment could be setup, using [jsdom](https://www.npmjs.com/package/jsdom) if needed.
@sebmarkbage
Copy link
Collaborator

@spicyj Should we have a custom transaction for shallow rendering? (Eventually we should get rid of transactions completely.)

@sophiebits
Copy link
Collaborator

Yeah maybe.

@glenjamin
Copy link
Contributor Author

For reference for future readers of this bug, the simplest current workaround I'm aware of is to do something like chantastic/react-media-object@3361126

Basically, create a global document object to silence the warning before any tests are run.

global.document = {}

@robertknight
Copy link
Contributor

getActiveElement was changed a couple of weeks ago to allow a null document - facebook/fbjs@2364a84 . If there is nothing else in setState that assumes the presence of a DOM, this might be fixed.

@graue
Copy link
Contributor

graue commented Jan 18, 2016

Good call, @robertknight. Looks like this was fixed in 0.14.0 (not RCs), broken in 0.14.4 (the fix was accidentally reverted upstream in fbjs), and fixed again in 0.14.6. I just tried @glenjamin's example with 0.14.6 and it's working (prints baz).

@graue graue closed this as completed Jan 18, 2016
@robertknight
Copy link
Contributor

@graue - I don't think we should close this until there is a regression test for it. I might be able to have a look at that.

@sophiebits
Copy link
Collaborator

Yeah, let's at least have a test.

@sophiebits sophiebits reopened this Jan 19, 2016
@graue graue changed the title setState in shallowRender fails due to missing document Need a test for setState in shallow render Jan 19, 2016
igorsantos07 added a commit to igorsantos07/redux that referenced this issue Mar 1, 2016
Updating docs regarding React updates on Shallow Rendering: facebook/react#4019 (comment)
igorsantos07 added a commit to igorsantos07/redux that referenced this issue Mar 1, 2016
Updating docs regarding React updates on Shallow Rendering: facebook/react#4019 (comment)
@Rybadour
Copy link

FYI: I found this issue while investigating why I was getting warnings about setState even though shallow rendering was working perfectly fine. Could you look into removing the warnings about using setState during shallow rendering.

Warning: setState(...): Can only update a mounted or mounting component. This usually means you called setState() on an unmounted component. This is a no-op.

Thanks!

@stefanscript
Copy link

stefanscript commented Mar 21, 2017

this decided to happen for me today. not fun :( .... even though worked fine till now
ReferenceError: document is not defined at getActiveElement (node_modules/fbjs/lib/getActiveElement.js:27:16)

narrowed it down to the same setState when shallow rendering.

@sebmarkbage
Copy link
Collaborator

@stefman1 That was a bug in a fbjs release. You can update to the new version. https://twitter.com/intelligibabble/status/844238291335458816

@gaearon
Copy link
Collaborator

gaearon commented Jan 2, 2018

We have a bunch of tests using setState with shallow renderer now. Also the new shallow renderer doesn't use getActiveElement() anyway.

@gaearon gaearon closed this as completed Jan 2, 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