invokeGuardedCallback should work when nested
Added a guard in the global error event listener to prevent nested errors from being captured higher up the stack. Also found a bug where the DEV version of invokeGuardedCallback would infinite loop if there were nested invocations with the same name. Fixed by appending the depth to the fake event name. (I'm actually not sure why this is necessary.)
Andrew Clark committed
Feb 8, 2017 at 17:11 UTC
13d45742281a2bcb478357e7c29b6db8c9d2c524
3 files changed
+43
-8
scripts/fiber/tests-passing.txt
+2
@@ -1676,6 +1676,8 @@ src/renderers/shared/utils/__tests__/ReactErrorUtils-test.js
1676
* should call the callback with the provided context
1677
* should return a caught error
1678
* should return null if no error is thrown
1679
+* can nest with same debug name
1680
+* does not return nested errors
1681
* should use invokeGuardedCallbackProd in production
1682
1683
src/renderers/shared/utils/__tests__/accumulateInto-test.js
src/renderers/shared/utils/ReactErrorUtils.js
+13
-8
@@ -97,10 +97,7 @@ if (__DEV__) {
97
typeof document.createEvent === 'function') {
98
99
const fakeNode = document.createElement('react');
100
- let fakeEventError = null;
101
- const onFakeEventError = function(event) {
102
- fakeEventError = event.error;
103
- };
100
+ let depth = 0;
101
102
ReactErrorUtils.invokeGuardedCallback = function(
103
name,
@@ -113,11 +110,20 @@ if (__DEV__) {
110
e,
111
f
112
) {
113
+ depth++;
114
+ const thisDepth = depth;
115
const funcArgs = Array.prototype.slice.call(arguments, 3);
116
const boundFunc = function() {
117
func.apply(context, funcArgs);
118
};
120
- const evtType = 'react-' + (name ? name : 'invokeguardedcallback');
119
+ let fakeEventError = null;
120
+ const onFakeEventError = function(event) {
121
+ // Don't capture nested errors
122
+ if (depth === thisDepth) {
123
+ fakeEventError = event.error;
124
+ }
125
+ };
126
+ const evtType = `react-${name ? name : 'invokeguardedcallback'}-${depth}`;
127
window.addEventListener('error', onFakeEventError);
128
fakeNode.addEventListener(evtType, boundFunc, false);
129
const evt = document.createEvent('Event');
@@ -125,9 +131,8 @@ if (__DEV__) {
131
fakeNode.dispatchEvent(evt);
132
fakeNode.removeEventListener(evtType, boundFunc, false);
133
window.removeEventListener('error', onFakeEventError);
128
- const returnVal = fakeEventError;
129
- fakeEventError = null;
130
- return returnVal;
134
+ depth--;
135
+ return fakeEventError;
136
};
137
}
138
}
src/renderers/shared/utils/__tests__/ReactErrorUtils-test.js
+28
@@ -89,6 +89,34 @@ describe('ReactErrorUtils', () => {
89
expect(returnValue).toBe(null);
90
});
91
92
+ it('can nest with same debug name', () => {
93
+ const err1 = new Error();
94
+ let err2;
95
+ const err3 = new Error();
96
+ const err4 = ReactErrorUtils.invokeGuardedCallback('foo', function() {
97
+ err2 = ReactErrorUtils.invokeGuardedCallback('foo', function() {
98
+ throw err1;
99
+ }, null);
100
+ throw err3;
101
+ }, null);
102
+
103
+ expect(err2).toBe(err1);
104
+ expect(err4).toBe(err3);
105
+ });
106
+
107
+ it('does not return nested errors', () => {
108
+ const err1 = new Error();
109
+ let err2;
110
+ const err3 = ReactErrorUtils.invokeGuardedCallback('foo', function() {
111
+ err2 = ReactErrorUtils.invokeGuardedCallback('foo', function() {
112
+ throw err1;
113
+ }, null);
114
+ }, null);
115
+
116
+ expect(err3).toBe(null); // Returns null because inner error was already captured
117
+ expect(err2).toBe(err1);
118
+ });
119
+
120
it('should use invokeGuardedCallbackProd in production', () => {
121
expect(ReactErrorUtils.invokeGuardedCallback).not.toEqual(
122
ReactErrorUtils.invokeGuardedCallbackProd