Remove feature test from invokeGuardedCallbackDev (#10283)
The critical semantics are resilient to browser flakiness, so we don't need this feature test. Also added comments explaining how invokeGuardedCallback dev works.
Andrew Clark committed
Jul 26, 2017 at 10:01 UTC
6c66d38d2549d14a3775a6a0687be6375967d009
3 files changed
+90
-66
scripts/jest/fiber.setup.js
-1
@@ -12,7 +12,6 @@ jest.mock('ReactFeatureFlags', () => {
12
const flags = require.requireActual('ReactFeatureFlags');
13
return Object.assign({}, flags, {
14
disableNewFiberFeatures: true,
15
- forceInvokeGuardedCallbackDev: true,
15
});
16
});
17
jest.mock('ReactNativeFeatureFlags', () => {
src/renderers/shared/utils/ReactErrorUtils.js
+90
-63
@@ -36,6 +36,10 @@ const ReactErrorUtils = {
36
/**
37
* Call a function while guarding against errors that happens within it.
38
* Returns an error if it throws, otherwise null.
39
+ *
40
+ * In production, this is implemented using a try-catch. The reason we don't
41
+ * use a try-catch directly is so that we can swap out a different
42
+ * implementation in DEV mode.
43
*
44
* @param {String} name of the guard to use for logging or debugging
45
* @param {Function} func The function to invoke
@@ -59,6 +63,7 @@ const ReactErrorUtils = {
63
/**
64
* Same as invokeGuardedCallback, but instead of returning an error, it stores
65
* it in a global so it can be rethrown by `rethrowCaughtError` later.
66
+ * TODO: See if _caughtError and _rethrowError can be unified.
67
*
68
* @param {String} name of the guard to use for logging or debugging
69
* @param {Function} func The function to invoke
@@ -127,20 +132,33 @@ let invokeGuardedCallback = function(name, func, context, a, b, c, d, e, f) {
132
};
133
134
if (__DEV__) {
130
- const ReactFeatureFlags = require('ReactFeatureFlags');
135
+ // In DEV mode, we swap out invokeGuardedCallback for a special version
136
+ // that plays more nicely with the browser's DevTools. The idea is to preserve
137
+ // "Pause on exceptions" behavior. Because React wraps all user-provided
138
+ // functions in invokeGuardedCallback, and the production version of
139
+ // invokeGuardedCallback uses a try-catch, all user exceptions are treated
140
+ // like caught exceptions, and the DevTools won't pause unless the developer
141
+ // takes the extra step of enabling pause on caught exceptions. This is
142
+ // untintuitive, though, because even though React has caught the error, from
143
+ // the developer's perspective, the error is uncaught.
144
+ //
145
+ // To preserve the expected "Pause on exceptions" behavior, we don't use a
146
+ // try-catch in DEV. Instead, we synchronously dispatch a fake event to a fake
147
+ // DOM node, and call the user-provided callback from inside an event handler
148
+ // for that fake event. If the callback throws, the error is "captured" using
149
+ // a global event handler. But because the error happens in a different
150
+ // event loop context, it does not interrupt the normal program flow.
151
+ // Effectively, this gives us try-catch behavior without actually using
152
+ // try-catch. Neat!
153
154
+ // Check that the browser supports the APIs we need to implement our special
155
+ // DEV version of invokeGuardedCallback
156
if (
157
typeof window !== 'undefined' &&
158
typeof window.dispatchEvent === 'function' &&
159
typeof document !== 'undefined' &&
160
typeof document.createEvent === 'function'
161
) {
138
- let preventDefault = true;
139
-
140
- /**
141
- * To help development we can get better devtools integration by simulating a
142
- * real browser event.
143
- */
162
const fakeNode = document.createElement('react');
163
let depth = 0;
164
@@ -155,30 +173,74 @@ if (__DEV__) {
173
e,
174
f,
175
) {
158
- depth++;
159
-
160
- let error;
176
+ // Keeps track of whether the user-provided callback threw an error. We
177
+ // set this to true at the beginning, then set it to false right after
178
+ // calling the function. If the function errors, `didError` will never be
179
+ // set to false. This strategy works even if the browser is flaky and
180
+ // fails to call our global error handler, because it doesn't rely on
181
+ // the error event at all.
182
let didError = true;
183
+
184
+ // Create an event handler for our fake event. We will synchronously
185
+ // dispatch our fake event using `dispatchEvent`. Inside the handler, we
186
+ // call the user-provided callback.
187
const funcArgs = Array.prototype.slice.call(arguments, 3);
163
- const boundFunc = function() {
188
+ function callCallback() {
189
func.apply(context, funcArgs);
190
didError = false;
166
- };
167
- const onFakeEventError = function(event) {
191
+ }
192
+
193
+ // Create a global error event handler. We use this to capture the value
194
+ // that was thrown. It's possible that this error handler will fire more
195
+ // than once; for example, if non-React code also calls `dispatchEvent`
196
+ // and a handler for that event throws. We should be resilient to most of
197
+ // those cases. Even if our error event handler fires more than once, the
198
+ // last error event is always used. If the callback actually does error,
199
+ // we know that the last error event is the correct one, because it's not
200
+ // possible for anything else to have happened in between our callback
201
+ // erroring and the code that follows the `dispatchEvent` call below. If
202
+ // the callback doesn't error, but the error event was fired, we know to
203
+ // ignore it because `didError` will be false, as described above.
204
+ let error;
205
+ // Use this to track whether the error event is ever called.
206
+ let didSetError = false;
207
+
208
+ function onError(event) {
209
error = event.error;
169
- if (preventDefault) {
170
- event.preventDefault();
171
- }
172
- };
210
+ didSetError = true;
211
+ }
212
213
+ // Create a fake event type. We add `depth` to the event name so that
214
+ // nested `invokeGuardedCallback` calls do not clash. Otherwise, a nested
215
+ // call would trigger the fake event handlers of any call higher in
216
+ // the stack.
217
+ depth++;
218
const evtType = `react-${name ? name : 'invokeguardedcallback'}-${depth}`;
175
- window.addEventListener('error', onFakeEventError);
176
- fakeNode.addEventListener(evtType, boundFunc, false);
177
- const evt = document.createEvent('Event');
219
220
+ // Attach our event handlers
221
+ window.addEventListener('error', onError);
222
+ fakeNode.addEventListener(evtType, callCallback, false);
223
+
224
+ // Synchronously dispatch our fake event. If the user-provided function
225
+ // errors, it will trigger our global error handler.
226
+ const evt = document.createEvent('Event');
227
evt.initEvent(evtType, false, false);
228
fakeNode.dispatchEvent(evt);
229
+
230
if (didError) {
231
+ if (!didSetError) {
232
+ // The callback errored, but the error event never fired.
233
+ error = new Error(
234
+ 'An error was thrown inside one of your components, but React ' +
235
+ "doesn't know what it was. This is likely due to browser " +
236
+ 'flakiness. React does its best to preserve the "Pause on ' +
237
+ 'exceptions" behavior of the DevTools, which requires some ' +
238
+ "DEV-mode only tricks. It's possible that these don't work in " +
239
+ 'your browser. Try triggering the error in production mode, ' +
240
+ 'or switching to a modern browser. If you suspect that this is ' +
241
+ 'actually an issue with React, please file an issue.',
242
+ );
243
+ }
244
ReactErrorUtils._hasCaughtError = true;
245
ReactErrorUtils._caughtError = error;
246
} else {
@@ -186,53 +248,18 @@ if (__DEV__) {
248
ReactErrorUtils._caughtError = null;
249
}
250
189
- fakeNode.removeEventListener(evtType, boundFunc, false);
190
- window.removeEventListener('error', onFakeEventError);
251
+ // Remove our event listeners
252
+ fakeNode.removeEventListener(evtType, callCallback, false);
253
+ window.removeEventListener('error', onError);
254
255
+ // Decrement the depth. This isn't strictly necessary, because we could
256
+ // just keep incrementing the number; the only requirement is that no two
257
+ // calls to `invokeGuardedCallback` use the same event type at the same
258
+ // time. But this does prevent `depth` from increasing boundlessly.
259
depth--;
260
};
261
195
- // Feature test the development version of invokeGuardedCallback
196
- // before enabling.
197
- let useInvokeGuardedCallbackDev;
198
- if (ReactFeatureFlags.forceInvokeGuardedCallbackDev) {
199
- // jsdom doesn't handle throwing null correctly (it fails when attempting
200
- // to access the 'message' property) but we need the ability to test it.
201
- // We use a feature flag to override the default feature test.
202
- useInvokeGuardedCallbackDev = true;
203
- } else {
204
- try {
205
- const err = new Error('test');
206
- invokeGuardedCallbackDev(
207
- null,
208
- () => {
209
- throw err;
210
- },
211
- null,
212
- );
213
- const A = ReactErrorUtils.clearCaughtError();
214
-
215
- invokeGuardedCallbackDev(
216
- null,
217
- () => {
218
- throw null;
219
- },
220
- null,
221
- );
222
- const B = ReactErrorUtils.clearCaughtError();
223
-
224
- if (A === err && B === null) {
225
- useInvokeGuardedCallbackDev = true;
226
- }
227
- } catch (e) {
228
- useInvokeGuardedCallbackDev = false;
229
- }
230
- }
231
-
232
- if (useInvokeGuardedCallbackDev) {
233
- invokeGuardedCallback = invokeGuardedCallbackDev;
234
- preventDefault = false;
235
- }
262
+ invokeGuardedCallback = invokeGuardedCallbackDev;
263
}
264
}
265
src/renderers/shared/utils/ReactFeatureFlags.js
-2
@@ -15,8 +15,6 @@
15
var ReactFeatureFlags = {
16
disableNewFiberFeatures: false,
17
enableAsyncSubtreeAPI: false,
18
- // We set this to true when running unit tests
19
- forceInvokeGuardedCallbackDev: false,
18
};
19
20
module.exports = ReactFeatureFlags;