-
-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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
npm test: setState in api callback: Cannot read property 'createEvent' of undefined #3482
Comments
getaround: check NODE_ENV componentDidMount() {
if (process.env.NODE_ENV === 'test') return // TODO 171121: https://github.com/facebookincubator/create-react-app/issues/3482 |
if you end up mocking the fetch call, all should be well! I was starting a project from scratch, and ended up here with the exact same error. then I saw I hadn't mocked out my fetch call! import 'isomorphic-fetch'
import fetchMock from 'fetch-mock'
import App from './App'
import { mockData } from 'data/fixtures'
describe('<App />', () => {
afterEach(() => {
fetchMock.reset()
fetchMock.restore()
})
it('renders without crashing', () => {
fetchMock.getOnce('/initialFetchCall/', mockData)
mount(
<App />
)
})
}) I am using enzyme 3, react 16 |
The problem is your test is now asynchronous but you don't wait for it to finish. So by the time the callback runs, the environment is already cleaned up. I think the right thing to do for us would be to explicitly unmount the component at the end of the test. For example: it('renders without crashing', () => {
const div = document.createElement('div');
ReactDOM.render(<App />, div);
ReactDOM.unmountComponentAtNode(div);
}); This would ensure only the initial render is being tested. You'd still have the problem of a "dead callback" because you never actually cancel it. But you have this problem anyway regardless of tests. If a component starts some work, class App extends Component {
state = {};
isMounted = false;
async componentDidMount() {
this.isMounted = true;
try {
await fetch('/proxiedApi');
} catch(e) {
if (this.isMounted) { // <--- don't call setState when unmounted
this.setState({e});
}
}
}
componentWillUnmount() {
this.isMounted = false; // <--- reset it on unmounting
}
render() {
// ...
}
} This solution is not ideal but will work. A better one would be to actually cancel the |
I merged #3511. If you make an equivalent change in your test, you can then fix your component to not do anything if it gets unmounted. I'll also look if we can provide a better warning on the React side when this happens. |
Here's a PR for React that should provide a better message for this, if merged. facebook/react#11677 |
For those of us, like @Timer that like to understand root causes, here is that: When async is used in a React component, the test for that component should be async, too (async is almost always the answer…) In this case, the test needs to be kept alive until both render and the promise completes, so the test needs to not only be async, but also have access to the promise. componentDidMount does not have a return value, but we can return the promise, so change the App.js line to: return this.myFetch().catch(e => this.setState({e})) Then make the test asynchronous and instrument componentDidMount: App.test.js: it('renders without crashing', async () => {
// instrument
const {prototype} = App
const {componentDidMount} = prototype
let promise
prototype.componentDidMount = function mock() {
promise = componentDidMount.apply(this) // this is the App instance
}
const div = document.createElement('div');
await new Promise((resolve, reject) => ReactDOM.render(<App />, div, resolve))
// wait for the promise to conclude
expect(promise).toBeInstanceOf(Promise, 'Instrumenting App instance failed')
await promise
// remove instrumentation
Object.assign(prototype, {componentDidMount})
}) As we all know, reactDOM.render has a callback and should be invoked with await To make these kinds of things less error prone, the great Create React Project could make the test async to begin with |
…and for Jestizens to use Jest when mocking a Component method: it('renders without crashing', async () => {
const {prototype} = App
const componentMethodName = 'componentDidMount'
const spy = jest.spyOn(prototype, componentMethodName)
let promise
// 171201: jest.spyOn cannot inspect return values https://github.com/facebook/jest/issues/3821
// We must therefore mock and intercept componentDidMount invocations to get the return value
// Jest does not allow direct access to the spiedMethod function value
// Here’s how to invoke the spied method, though
const invokeSpiedMethod = spy.getMockImplementation()
spy.mockImplementation(function mock(...args) {
// inside the mock implementation, the this value is the App instance
// the App instance’s method has been mocked and cannot be invoked
return promise = invokeSpiedMethod.apply(this, args)
})
const div = document.createElement('div');
await new Promise((resolve, reject) => ReactDOM.render(<App />, div, resolve))
// wait for the promise to conclude
expect(promise).toBeInstanceOf(Promise, 'Instrumenting App instance failed')
await promise
spy.mockReset()
spy.mockRestore()
}) Just in case: |
I’m not sure if you read my comments above, but I suggested a way to solve this problem without any mocking or making your component async. Just unmount it in the test, and make sure any async work is canceled on unmounting (which is a good idea anyway). |
I saw that: I learned that state should be an ECMAScript instance property. That way you do not have to type out the constructor. I wanted to leave readers with the root cause that is: Then I wanted to understand what the Jesty and @facebook’ey way would be to do the async version of the test, ie. mocking and intercepting a ReactJS component method. I discovered there that, for example, Jest cannot presently intercept a component constructor, due to troubles with invocations using the new operator. If that worked, the tested component could be extended which might be useful. It is more for the future than this particular problem since I use async a lot. Here’s a general learning you will like: I was flattered to have @gaearon look at my ticket. |
And here is how to extend a React component during testing in App.test.js: import React from 'react'
import ReactDOM from 'react-dom'
import App, * as AppImport from './App'
it('renders without crashing', async () => {
class AppTest extends App {
static promise
componentDidMount(...args) {
return AppTest.promise = super.componentDidMount(...args)
}
}
const App0 = App
AppImport.default = AppTest
const div = document.createElement('div');
await new Promise((resolve, reject) => ReactDOM.render(<App />, div, resolve))
// wait for the promise to conclude
const promise = AppTest.promise
expect(promise).toBeInstanceOf(Promise, 'Instrumenting App instance failed')
await promise
AppImport.default = App0
}) There might be additional trickery required to make this work more broadly I noticed that async tests basically breaks error reporting, errors get much harder to troubleshoot Create React App is extremely helpful in illustrating solutions with working code. |
By the way as far as I can see this doesn't crash the test runner anymore on |
Is this a bug report?
Yes
Can you also reproduce the problem with npm 4.x?
I am on 5.5.1 and it’s not npm related
Environment
node -v
v9.2.0
npm ls react-scripts
react-app@0.1.0 /opt/foxyboy/sw/pri/code-samples/react-test-error
└── react-scripts@1.0.17
echo $(lsb_release --description --codename --short)
Ubuntu 17.10 artful
Root Cause
I used npm t on a component using unmocked fetch, which of course fails.
However, updating state in the catch clause causes a render with the global document value undefined
node_modules/react-dom/cjs/react-dom.development.js:577
is this a bug?
Steps to Reproduce
Expected Behavior
Actual Behavior
TypeError: Cannot read property 'createEvent' of undefined
The text was updated successfully, but these errors were encountered: