Skip to content

Commit 940f48b

Browse files
sebmarkbagesebmarkbage
authored
Avoid passing custom stacks to console.error (#18685)
* Detect double stacks in the new format in tests * Remove unnecessary uses of getStackByFiberInDevAndProd These all execute in the right execution context already. * Set the debug fiber around the cases that don't have an execution context * Remove stack detection in our console log overrides We never pass custom stacks as part of the args anymore. * Bonus: Don't append getStackAddendum to invariants We print component stacks for every error anyway so this is just duplicate information.
1 parent ff431b7 commit 940f48b

20 files changed

+325
-404
lines changed

‎packages/react-dom/src/__tests__/ReactComponent-test.js

Lines changed: 18 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,6 @@ let ReactDOMServer;
1515
let ReactTestUtils;
1616

1717
describe('ReactComponent', () => {
18-
function normalizeCodeLocInfo(str) {
19-
return (
20-
str &&
21-
str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function(m, name) {
22-
return '\n in ' + name + ' (at **)';
23-
})
24-
);
25-
}
26-
2718
beforeEach(() => {
2819
React = require('react');
2920
ReactDOM = require('react-dom');
@@ -470,20 +461,11 @@ describe('ReactComponent', () => {
470461
};
471462
const element = <div>{[children]}</div>;
472463
const container = document.createElement('div');
473-
let ex;
474-
try {
464+
expect(() => {
475465
ReactDOM.render(element, container);
476-
} catch (e) {
477-
ex = e;
478-
}
479-
expect(ex).toBeDefined();
480-
expect(normalizeCodeLocInfo(ex.message)).toBe(
481-
'Objects are not valid as a React child (found: object with keys {x, y, z}).' +
482-
(__DEV__
483-
? ' If you meant to render a collection of children, use ' +
484-
'an array instead.' +
485-
'\n in div (at **)'
486-
: ''),
466+
}).toThrowError(
467+
'Objects are not valid as a React child (found: object with keys {x, y, z}). ' +
468+
'If you meant to render a collection of children, use an array instead.',
487469
);
488470
});
489471

@@ -499,21 +481,12 @@ describe('ReactComponent', () => {
499481
}
500482
}
501483
const container = document.createElement('div');
502-
let ex;
503-
try {
484+
expect(() => {
504485
ReactDOM.render(<Foo />, container);
505-
} catch (e) {
506-
ex = e;
507-
}
508-
expect(ex).toBeDefined();
509-
expect(normalizeCodeLocInfo(ex.message)).toBe(
486+
}).toThrowError(
510487
'Objects are not valid as a React child (found: object with keys {a, b, c}).' +
511-
(__DEV__
512-
? ' If you meant to render a collection of children, use ' +
513-
'an array instead.\n' +
514-
' in div (at **)\n' +
515-
' in Foo (at **)'
516-
: ''),
488+
' If you meant to render a collection of children, use an array ' +
489+
'instead.',
517490
);
518491
});
519492

@@ -524,20 +497,12 @@ describe('ReactComponent', () => {
524497
z: <span />,
525498
};
526499
const element = <div>{[children]}</div>;
527-
let ex;
528-
try {
500+
expect(() => {
529501
ReactDOMServer.renderToString(element);
530-
} catch (e) {
531-
ex = e;
532-
}
533-
expect(ex).toBeDefined();
534-
expect(normalizeCodeLocInfo(ex.message)).toBe(
535-
'Objects are not valid as a React child (found: object with keys {x, y, z}).' +
536-
(__DEV__
537-
? ' If you meant to render a collection of children, use ' +
538-
'an array instead.' +
539-
'\n in div (at **)'
540-
: ''),
502+
}).toThrowError(
503+
'Objects are not valid as a React child (found: object with keys {x, y, z}). ' +
504+
'If you meant to render a collection of children, use ' +
505+
'an array instead.',
541506
);
542507
});
543508

@@ -553,21 +518,12 @@ describe('ReactComponent', () => {
553518
}
554519
}
555520
const container = document.createElement('div');
556-
let ex;
557-
try {
521+
expect(() => {
558522
ReactDOMServer.renderToString(<Foo />, container);
559-
} catch (e) {
560-
ex = e;
561-
}
562-
expect(ex).toBeDefined();
563-
expect(normalizeCodeLocInfo(ex.message)).toBe(
564-
'Objects are not valid as a React child (found: object with keys {a, b, c}).' +
565-
(__DEV__
566-
? ' If you meant to render a collection of children, use ' +
567-
'an array instead.\n' +
568-
' in div (at **)\n' +
569-
' in Foo (at **)'
570-
: ''),
523+
}).toThrowError(
524+
'Objects are not valid as a React child (found: object with keys {a, b, c}). ' +
525+
'If you meant to render a collection of children, use ' +
526+
'an array instead.',
571527
);
572528
});
573529

‎packages/react-dom/src/__tests__/ReactDOMComponent-test.js

Lines changed: 12 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,6 @@ describe('ReactDOMComponent', () => {
1616
let ReactDOMServer;
1717
const ReactFeatureFlags = require('shared/ReactFeatureFlags');
1818

19-
function normalizeCodeLocInfo(str) {
20-
return (
21-
str &&
22-
str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function(m, name) {
23-
return '\n in ' + name + ' (at **)';
24-
})
25-
);
26-
}
27-
2819
beforeEach(() => {
2920
jest.resetModules();
3021
React = require('react');
@@ -1320,36 +1311,24 @@ describe('ReactDOMComponent', () => {
13201311

13211312
it('should throw on children for void elements', () => {
13221313
const container = document.createElement('div');
1323-
let caughtErr;
1324-
try {
1314+
expect(() => {
13251315
ReactDOM.render(<input>children</input>, container);
1326-
} catch (err) {
1327-
caughtErr = err;
1328-
}
1329-
expect(caughtErr).not.toBe(undefined);
1330-
expect(normalizeCodeLocInfo(caughtErr.message)).toContain(
1316+
}).toThrowError(
13311317
'input is a void element tag and must neither have `children` nor ' +
1332-
'use `dangerouslySetInnerHTML`.' +
1333-
(__DEV__ ? '\n in input (at **)' : ''),
1318+
'use `dangerouslySetInnerHTML`.',
13341319
);
13351320
});
13361321

13371322
it('should throw on dangerouslySetInnerHTML for void elements', () => {
13381323
const container = document.createElement('div');
1339-
let caughtErr;
1340-
try {
1324+
expect(() => {
13411325
ReactDOM.render(
13421326
<input dangerouslySetInnerHTML={{__html: 'content'}} />,
13431327
container,
13441328
);
1345-
} catch (err) {
1346-
caughtErr = err;
1347-
}
1348-
expect(caughtErr).not.toBe(undefined);
1349-
expect(normalizeCodeLocInfo(caughtErr.message)).toContain(
1329+
}).toThrowError(
13501330
'input is a void element tag and must neither have `children` nor ' +
1351-
'use `dangerouslySetInnerHTML`.' +
1352-
(__DEV__ ? '\n in input (at **)' : ''),
1331+
'use `dangerouslySetInnerHTML`.',
13531332
);
13541333
});
13551334

@@ -1461,18 +1440,11 @@ describe('ReactDOMComponent', () => {
14611440
}
14621441

14631442
const container = document.createElement('div');
1464-
let caughtErr;
1465-
try {
1443+
expect(() => {
14661444
ReactDOM.render(<X />, container);
1467-
} catch (err) {
1468-
caughtErr = err;
1469-
}
1470-
1471-
expect(caughtErr).not.toBe(undefined);
1472-
expect(normalizeCodeLocInfo(caughtErr.message)).toContain(
1445+
}).toThrowError(
14731446
'input is a void element tag and must neither have `children` ' +
1474-
'nor use `dangerouslySetInnerHTML`.' +
1475-
(__DEV__ ? '\n in input (at **)' + '\n in X (at **)' : ''),
1447+
'nor use `dangerouslySetInnerHTML`.',
14761448
);
14771449
});
14781450

@@ -1627,19 +1599,12 @@ describe('ReactDOMComponent', () => {
16271599
}
16281600
}
16291601

1630-
let caughtErr;
1631-
try {
1602+
expect(() => {
16321603
ReactDOM.render(<Animal />, container);
1633-
} catch (err) {
1634-
caughtErr = err;
1635-
}
1636-
1637-
expect(caughtErr).not.toBe(undefined);
1638-
expect(normalizeCodeLocInfo(caughtErr.message)).toContain(
1604+
}).toThrowError(
16391605
'The `style` prop expects a mapping from style properties to values, ' +
16401606
"not a string. For example, style={{marginRight: spacing + 'em'}} " +
1641-
'when using JSX.' +
1642-
(__DEV__ ? '\n in div (at **)' + '\n in Animal (at **)' : ''),
1607+
'when using JSX.',
16431608
);
16441609
});
16451610

‎packages/react-dom/src/__tests__/ReactServerRendering-test.js

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,6 @@ let ReactCurrentDispatcher;
1717
const enableSuspenseServerRenderer = require('shared/ReactFeatureFlags')
1818
.enableSuspenseServerRenderer;
1919

20-
function normalizeCodeLocInfo(str) {
21-
return (
22-
str &&
23-
str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function(m, name) {
24-
return '\n in ' + name + ' (at **)';
25-
})
26-
);
27-
}
28-
2920
describe('ReactDOMServer', () => {
3021
beforeEach(() => {
3122
jest.resetModules();
@@ -172,17 +163,11 @@ describe('ReactDOMServer', () => {
172163
});
173164

174165
it('should throw prop mapping error for an <iframe /> with invalid props', () => {
175-
let caughtErr;
176-
try {
166+
expect(() => {
177167
ReactDOMServer.renderToString(<iframe style="border:none;" />);
178-
} catch (err) {
179-
caughtErr = err;
180-
}
181-
expect(caughtErr).not.toBe(undefined);
182-
expect(normalizeCodeLocInfo(caughtErr.message)).toContain(
168+
}).toThrowError(
183169
'The `style` prop expects a mapping from style properties to values, not ' +
184-
"a string. For example, style={{marginRight: spacing + 'em'}} when using JSX." +
185-
(__DEV__ ? '\n in iframe (at **)' : ''),
170+
"a string. For example, style={{marginRight: spacing + 'em'}} when using JSX.",
186171
);
187172
});
188173

‎packages/react-dom/src/shared/assertValidProps.js

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,11 @@
66
*/
77

88
import invariant from 'shared/invariant';
9-
// TODO: We can remove this if we add invariantWithStack()
10-
// or add stack by default to invariants where possible.
11-
import ReactSharedInternals from 'shared/ReactSharedInternals';
129

1310
import voidElementTags from './voidElementTags';
1411

1512
const HTML = '__html';
1613

17-
let ReactDebugCurrentFrame = null;
18-
if (__DEV__) {
19-
ReactDebugCurrentFrame = ReactSharedInternals.ReactDebugCurrentFrame;
20-
}
21-
2214
function assertValidProps(tag: string, props: ?Object) {
2315
if (!props) {
2416
return;
@@ -28,9 +20,8 @@ function assertValidProps(tag: string, props: ?Object) {
2820
invariant(
2921
props.children == null && props.dangerouslySetInnerHTML == null,
3022
'%s is a void element tag and must neither have `children` nor ' +
31-
'use `dangerouslySetInnerHTML`.%s',
23+
'use `dangerouslySetInnerHTML`.',
3224
tag,
33-
__DEV__ ? ReactDebugCurrentFrame.getStackAddendum() : '',
3425
);
3526
}
3627
if (props.dangerouslySetInnerHTML != null) {
@@ -64,8 +55,7 @@ function assertValidProps(tag: string, props: ?Object) {
6455
props.style == null || typeof props.style === 'object',
6556
'The `style` prop expects a mapping from style properties to values, ' +
6657
"not a string. For example, style={{marginRight: spacing + 'em'}} when " +
67-
'using JSX.%s',
68-
__DEV__ ? ReactDebugCurrentFrame.getStackAddendum() : '',
58+
'using JSX.',
6959
);
7060
}
7161

‎packages/react-dom/src/shared/sanitizeURL.js

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,8 @@
88
*/
99

1010
import invariant from 'shared/invariant';
11-
import ReactSharedInternals from 'shared/ReactSharedInternals';
1211
import {disableJavaScriptURLs} from 'shared/ReactFeatureFlags';
1312

14-
let ReactDebugCurrentFrame = ((null: any): {getStackAddendum(): string, ...});
15-
if (__DEV__) {
16-
ReactDebugCurrentFrame = ReactSharedInternals.ReactDebugCurrentFrame;
17-
}
18-
1913
// A javascript: URL can contain leading C0 control or \u0020 SPACE,
2014
// and any newline or tab are filtered out as if they're not part of the URL.
2115
// https://url.spec.whatwg.org/#url-parsing
@@ -34,8 +28,7 @@ function sanitizeURL(url: string) {
3428
if (disableJavaScriptURLs) {
3529
invariant(
3630
!isJavaScriptProtocol.test(url),
37-
'React has blocked a javascript: URL as a security precaution.%s',
38-
__DEV__ ? ReactDebugCurrentFrame.getStackAddendum() : '',
31+
'React has blocked a javascript: URL as a security precaution.',
3932
);
4033
} else if (__DEV__) {
4134
if (!didWarn && isJavaScriptProtocol.test(url)) {

‎packages/react-reconciler/src/ReactChildFiber.new.js

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ import {
4444
createFiberFromPortal,
4545
} from './ReactFiber.new';
4646
import {emptyRefsObject} from './ReactFiberClassComponent.new';
47-
import {getStackByFiberInDevAndProd} from './ReactFiberComponentStack';
4847
import {getCurrentFiberStackInDev} from './ReactCurrentFiber';
4948
import {isCompatibleFamilyForHotReloading} from './ReactFiberHotReloading.new';
5049
import {StrictMode} from './ReactTypeOfMode';
@@ -136,20 +135,18 @@ function coerceRef(
136135
'will be removed in a future major release. We recommend using ' +
137136
'useRef() or createRef() instead. ' +
138137
'Learn more about using refs safely here: ' +
139-
'https://fb.me/react-strict-mode-string-ref%s',
138+
'https://fb.me/react-strict-mode-string-ref',
140139
componentName,
141140
mixedRef,
142-
getStackByFiberInDevAndProd(returnFiber),
143141
);
144142
} else {
145143
console.error(
146144
'A string ref, "%s", has been found within a strict mode tree. ' +
147145
'String refs are a source of potential bugs and should be avoided. ' +
148146
'We recommend using useRef() or createRef() instead. ' +
149147
'Learn more about using refs safely here: ' +
150-
'https://fb.me/react-strict-mode-string-ref%s',
148+
'https://fb.me/react-strict-mode-string-ref',
151149
mixedRef,
152-
getStackByFiberInDevAndProd(returnFiber),
153150
);
154151
}
155152
didWarnAboutStringRefs[componentName] = true;
@@ -223,20 +220,14 @@ function coerceRef(
223220

224221
function throwOnInvalidObjectType(returnFiber: Fiber, newChild: Object) {
225222
if (returnFiber.type !== 'textarea') {
226-
let addendum = '';
227-
if (__DEV__) {
228-
addendum =
229-
' If you meant to render a collection of children, use an array ' +
230-
'instead.' +
231-
getCurrentFiberStackInDev();
232-
}
233223
invariant(
234224
false,
235-
'Objects are not valid as a React child (found: %s).%s',
225+
'Objects are not valid as a React child (found: %s). ' +
226+
'If you meant to render a collection of children, use an array ' +
227+
'instead.',
236228
Object.prototype.toString.call(newChild) === '[object Object]'
237229
? 'object with keys {' + Object.keys(newChild).join(', ') + '}'
238230
: newChild,
239-
addendum,
240231
);
241232
}
242233
}

0 commit comments

Comments
 (0)