From 1968ad6f94ed812223270ce29be7e848d4b30c56 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 27 Nov 2017 22:57:45 +0000 Subject: [PATCH 1/5] Warn if `document` is missing by the time invokeGuardedCallback runs in DEV --- .../react-dom/src/__tests__/ReactDOM-test.js | 41 +++++++++++++++++++ packages/shared/ReactErrorUtils.js | 11 +++++ 2 files changed, 52 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index dde2608bfa3..cd08297d4cb 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -372,4 +372,45 @@ describe('ReactDOM', () => { delete global.__REACT_DEVTOOLS_GLOBAL_HOOK__; } }); + + it('warns in DEV if jsdom is destroyed by the time setState() is called', () => { + spyOnDev(console, 'error'); + class App extends React.Component { + state = {x: 1}; + render() { + return
; + } + } + const container = document.createElement('div'); + const instance = ReactDOM.render(, container); + const documentDescriptor = Object.getOwnPropertyDescriptor( + global, + 'document', + ); + try { + // Emulate jsdom environment cleanup. + // This is rouhly what happens if the test finished and then + // an asynchronous callback tried to setState() after this. + delete global.document; + const fn = () => instance.setState({x: 2}); + if (__DEV__) { + expect(fn).toThrow('document is not defined'); + expect(console.error.calls.count()).toBe(1); + expect(console.error.calls.argsFor(0)[0]).toContain( + 'The `document` global was defined when React was initialized, but is not ' + + 'defined anymore. This can happen in a test environment if a component ' + + 'schedules an update from an asynchronous callback, but the test has already ' + + 'finished running. To solve this, you can either unmount the component at ' + + 'the end of your test (and ensure that any asynchronous operations get ' + + 'canceled in `componentWillUnmount`), or you can change the test itself ' + + 'to be asynchronous.', + ); + } else { + expect(fn).not.toThrow(); + } + } finally { + // Don't break other tests. + Object.defineProperty(global, 'document', documentDescriptor); + } + }); }); diff --git a/packages/shared/ReactErrorUtils.js b/packages/shared/ReactErrorUtils.js index cc59ee6fb4b..80501d04d98 100644 --- a/packages/shared/ReactErrorUtils.js +++ b/packages/shared/ReactErrorUtils.js @@ -8,6 +8,7 @@ */ import invariant from 'fbjs/lib/invariant'; +import warning from 'fbjs/lib/warning'; const ReactErrorUtils = { // Used by Fiber to simulate a try-catch. @@ -167,6 +168,16 @@ if (__DEV__) { e, f, ) { + warning( + typeof document !== 'undefined', + 'The `document` global was defined when React was initialized, but is not ' + + 'defined anymore. This can happen in a test environment if a component ' + + 'schedules an update from an asynchronous callback, but the test has already ' + + 'finished running. To solve this, you can either unmount the component at ' + + 'the end of your test (and ensure that any asynchronous operations get ' + + 'canceled in `componentWillUnmount`), or you can change the test itself ' + + 'to be asynchronous.', + ); // Keeps track of whether the user-provided callback threw an error. We // set this to true at the beginning, then set it to false right after // calling the function. If the function errors, `didError` will never be From b9f46538cfaca02be47ac46322f21a2d55909175 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 27 Nov 2017 23:24:29 +0000 Subject: [PATCH 2/5] Typo --- packages/react-dom/src/__tests__/ReactDOM-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index cd08297d4cb..7df81ef4047 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -389,7 +389,7 @@ describe('ReactDOM', () => { ); try { // Emulate jsdom environment cleanup. - // This is rouhly what happens if the test finished and then + // This is roughly what happens if the test finished and then // an asynchronous callback tried to setState() after this. delete global.document; const fn = () => instance.setState({x: 2}); From a3d29dd3b98444a2019e47ae0392a10dc4c82c36 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 29 Nov 2017 14:43:11 +0000 Subject: [PATCH 3/5] Add a comment --- packages/shared/ReactErrorUtils.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/shared/ReactErrorUtils.js b/packages/shared/ReactErrorUtils.js index 80501d04d98..0a7576e1236 100644 --- a/packages/shared/ReactErrorUtils.js +++ b/packages/shared/ReactErrorUtils.js @@ -168,6 +168,8 @@ if (__DEV__) { e, f, ) { + // If document doesn't exist we know for sure we will crash in this method later. + // So we show a warning explaining a potential cause. warning( typeof document !== 'undefined', 'The `document` global was defined when React was initialized, but is not ' + From c7817d257b11597ed87d0796daa9f7685a087eaf Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 29 Nov 2017 14:55:29 +0000 Subject: [PATCH 4/5] Use invariant() instead --- packages/react-dom/src/__tests__/ReactDOM-test.js | 6 ++---- packages/shared/ReactErrorUtils.js | 9 +++++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index 7df81ef4047..f10ebfb6dbc 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -373,7 +373,7 @@ describe('ReactDOM', () => { } }); - it('warns in DEV if jsdom is destroyed by the time setState() is called', () => { + it('throws in DEV if jsdom is destroyed by the time setState() is called', () => { spyOnDev(console, 'error'); class App extends React.Component { state = {x: 1}; @@ -394,9 +394,7 @@ describe('ReactDOM', () => { delete global.document; const fn = () => instance.setState({x: 2}); if (__DEV__) { - expect(fn).toThrow('document is not defined'); - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( + expect(fn).toThrow( 'The `document` global was defined when React was initialized, but is not ' + 'defined anymore. This can happen in a test environment if a component ' + 'schedules an update from an asynchronous callback, but the test has already ' + diff --git a/packages/shared/ReactErrorUtils.js b/packages/shared/ReactErrorUtils.js index 0a7576e1236..4affe5ac7c8 100644 --- a/packages/shared/ReactErrorUtils.js +++ b/packages/shared/ReactErrorUtils.js @@ -8,7 +8,6 @@ */ import invariant from 'fbjs/lib/invariant'; -import warning from 'fbjs/lib/warning'; const ReactErrorUtils = { // Used by Fiber to simulate a try-catch. @@ -168,9 +167,11 @@ if (__DEV__) { e, f, ) { - // If document doesn't exist we know for sure we will crash in this method later. - // So we show a warning explaining a potential cause. - warning( + // If document doesn't exist we know for sure we will crash in this method + // when we call document.createEvent(). However this can cause confusing + // errors: https://github.com/facebookincubator/create-react-app/issues/3482 + // So we preemptively throw with a better message instead. + invariant( typeof document !== 'undefined', 'The `document` global was defined when React was initialized, but is not ' + 'defined anymore. This can happen in a test environment if a component ' + From 2542fb7288e99d753a3f0012e0f95f64ee16c7b9 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 29 Nov 2017 15:42:36 +0000 Subject: [PATCH 5/5] Create event immediately for clarity --- packages/shared/ReactErrorUtils.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/shared/ReactErrorUtils.js b/packages/shared/ReactErrorUtils.js index 4affe5ac7c8..cbbfc81652b 100644 --- a/packages/shared/ReactErrorUtils.js +++ b/packages/shared/ReactErrorUtils.js @@ -181,6 +181,8 @@ if (__DEV__) { 'canceled in `componentWillUnmount`), or you can change the test itself ' + 'to be asynchronous.', ); + const evt = document.createEvent('Event'); + // Keeps track of whether the user-provided callback threw an error. We // set this to true at the beginning, then set it to false right after // calling the function. If the function errors, `didError` will never be @@ -236,7 +238,6 @@ if (__DEV__) { // Synchronously dispatch our fake event. If the user-provided function // errors, it will trigger our global error handler. - const evt = document.createEvent('Event'); evt.initEvent(evtType, false, false); fakeNode.dispatchEvent(evt);