Include component stack in more places, including SSR (#11284)
Include component stack in more places, including SSR (#11284) * Include component stack in more places, including SSR * Forbid including reconciler code into the server bundle * Tighten up the Flow annotation * Fix lint * Gosh Prettier
Dan Abramov committed
Oct 19, 2017 at 17:43 UTC
1a81be46258ef1f8173c7991b5cca79df6518e95
6 files changed
+88
-92
packages/react-dom/src/ReactDOMFiberComponent.js
+4
-4
@@ -61,7 +61,7 @@ var HTML = '__html';
61
62
var {Namespaces: {html: HTML_NAMESPACE}, getIntrinsicNamespace} = DOMNamespaces;
63
64
-var getStack = emptyFunction;
64
+var getStack = emptyFunction.thatReturnsArgument('');
65
66
if (__DEV__) {
67
getStack = getCurrentFiberStackAddendum;
@@ -562,7 +562,7 @@ var ReactDOMFiberComponent = {
562
props = rawProps;
563
}
564
565
- assertValidProps(tag, props, getCurrentFiberOwnerName);
565
+ assertValidProps(tag, props, getStack);
566
567
setInitialDOMProperties(
568
tag,
@@ -656,7 +656,7 @@ var ReactDOMFiberComponent = {
656
break;
657
}
658
659
- assertValidProps(tag, nextProps, getCurrentFiberOwnerName);
659
+ assertValidProps(tag, nextProps, getStack);
660
661
var propKey;
662
var styleName;
@@ -971,7 +971,7 @@ var ReactDOMFiberComponent = {
971
break;
972
}
973
974
- assertValidProps(tag, rawProps, getCurrentFiberOwnerName);
974
+ assertValidProps(tag, rawProps, getStack);
975
976
if (__DEV__) {
977
var extraAttributeNames: Set<string> = new Set();
packages/react-dom/src/server/ReactPartialRenderer.js
+8
-12
@@ -30,7 +30,7 @@ var omittedCloseTags = require('omittedCloseTags');
30
var isCustomComponent = require('isCustomComponent');
31
32
var toArray = React.Children.toArray;
33
-var emptyFunctionThatReturnsNull = emptyFunction.thatReturnsNull;
33
+var getStackAddendum = emptyFunction.thatReturnsArgument('');
34
35
if (__DEV__) {
36
var warning = require('fbjs/lib/warning');
@@ -80,9 +80,9 @@ if (__DEV__) {
80
currentDebugStack = null;
81
ReactDebugCurrentFrame.getCurrentStack = null;
82
};
83
- var getStackAddendum = function(): null | string {
83
+ getStackAddendum = function(): null | string {
84
if (currentDebugStack === null) {
85
- return null;
85
+ return '';
86
}
87
let stack = '';
88
let debugStack = currentDebugStack;
@@ -141,11 +141,7 @@ function createMarkupForStyles(styles) {
141
var styleValue = styles[styleName];
142
if (__DEV__) {
143
if (!isCustomProperty) {
144
- warnValidStyle(
145
- styleName,
146
- styleValue,
147
- () => '', // getCurrentFiberStackAddendum,
148
- );
144
+ warnValidStyle(styleName, styleValue, getStackAddendum);
145
}
146
}
147
if (styleValue != null) {
@@ -574,7 +570,7 @@ class ReactDOMServerRenderer {
570
ReactControlledValuePropTypes.checkPropTypes(
571
'input',
572
props,
577
- () => '', //getCurrentFiberStackAddendum
573
+ getStackAddendum,
574
);
575
576
if (
@@ -632,7 +628,7 @@ class ReactDOMServerRenderer {
628
ReactControlledValuePropTypes.checkPropTypes(
629
'textarea',
630
props,
635
- () => '', //getCurrentFiberStackAddendum
631
+ getStackAddendum,
632
);
633
if (
634
props.value !== undefined &&
@@ -693,7 +689,7 @@ class ReactDOMServerRenderer {
689
ReactControlledValuePropTypes.checkPropTypes(
690
'select',
691
props,
696
- () => '', // getCurrentFiberStackAddendum,
692
+ getStackAddendum,
693
);
694
695
for (var i = 0; i < valuePropNames.length; i++) {
@@ -785,7 +781,7 @@ class ReactDOMServerRenderer {
781
validatePropertiesInDevelopment(tag, props);
782
}
783
788
- assertValidProps(tag, props, emptyFunctionThatReturnsNull);
784
+ assertValidProps(tag, props, getStackAddendum);
785
786
var out = createOpenTagMarkup(
787
element.type,
packages/react-dom/src/shared/__tests__/ReactDOMComponent-test.js
+44
-40
@@ -954,50 +954,38 @@ describe('ReactDOMComponent', () => {
954
});
955
956
it('should warn against children for void elements', () => {
957
- var container = document.createElement('div');
958
-
959
- expect(function() {
957
+ const container = document.createElement('div');
958
+ let caughtErr;
959
+ try {
960
ReactDOM.render(<input>children</input>, container);
961
- }).toThrowError(
961
+ } catch (err) {
962
+ caughtErr = err;
963
+ }
964
+ expect(caughtErr).not.toBe(undefined);
965
+ expect(normalizeCodeLocInfo(caughtErr.message)).toContain(
966
'input is a void element tag and must neither have `children` nor ' +
967
'use `dangerouslySetInnerHTML`.',
968
+ '\n in input (at **)',
969
);
970
});
971
972
it('should warn against dangerouslySetInnerHTML for void elements', () => {
968
- var container = document.createElement('div');
969
-
970
- expect(function() {
973
+ const container = document.createElement('div');
974
+ let caughtErr;
975
+ try {
976
ReactDOM.render(
977
<input dangerouslySetInnerHTML={{__html: 'content'}} />,
978
container,
979
);
975
- }).toThrowError(
976
- 'input is a void element tag and must neither have `children` nor use ' +
977
- '`dangerouslySetInnerHTML`.',
978
- );
979
- });
980
-
981
- it('should include owner rather than parent in warnings', () => {
982
- var container = document.createElement('div');
983
-
984
- function Parent(props) {
985
- return props.children;
986
- }
987
- function Owner() {
988
- // We're using the input dangerouslySetInnerHTML invariant but the
989
- // exact error doesn't matter as long as we have a way to verify
990
- // that warnings and invariants contain owner rather than parent name.
991
- return (
992
- <Parent>
993
- <input dangerouslySetInnerHTML={{__html: 'content'}} />
994
- </Parent>
995
- );
980
+ } catch (err) {
981
+ caughtErr = err;
982
}
997
-
998
- expect(function() {
999
- ReactDOM.render(<Owner />, container);
1000
- }).toThrowError('\n\nThis DOM node was rendered by `Owner`.');
983
+ expect(caughtErr).not.toBe(undefined);
984
+ expect(normalizeCodeLocInfo(caughtErr.message)).toContain(
985
+ 'input is a void element tag and must neither have `children` nor ' +
986
+ 'use `dangerouslySetInnerHTML`.',
987
+ '\n in input (at **)',
988
+ );
989
});
990
991
it('should emit a warning once for a named custom component using shady DOM', () => {
@@ -1178,19 +1166,27 @@ describe('ReactDOMComponent', () => {
1166
expect(tracker.getValue()).toEqual('foo');
1167
});
1168
1181
- it('should warn for children on void elements', () => {
1169
+ it('should throw for children on void elements', () => {
1170
class X extends React.Component {
1171
render() {
1172
return <input>moo</input>;
1173
}
1174
}
1175
1188
- var container = document.createElement('div');
1189
- expect(function() {
1176
+ const container = document.createElement('div');
1177
+ let caughtErr;
1178
+ try {
1179
ReactDOM.render(<X />, container);
1191
- }).toThrowError(
1180
+ } catch (err) {
1181
+ caughtErr = err;
1182
+ }
1183
+
1184
+ expect(caughtErr).not.toBe(undefined);
1185
+ expect(normalizeCodeLocInfo(caughtErr.message)).toContain(
1186
'input is a void element tag and must neither have `children` ' +
1193
- 'nor use `dangerouslySetInnerHTML`.\n\nThis DOM node was rendered by `X`.',
1187
+ 'nor use `dangerouslySetInnerHTML`.' +
1188
+ '\n in input (at **)' +
1189
+ '\n in X (at **)',
1190
);
1191
});
1192
@@ -1303,12 +1299,20 @@ describe('ReactDOMComponent', () => {
1299
}
1300
}
1301
1306
- expect(function() {
1302
+ let caughtErr;
1303
+ try {
1304
ReactDOM.render(<Animal />, container);
1308
- }).toThrowError(
1305
+ } catch (err) {
1306
+ caughtErr = err;
1307
+ }
1308
+
1309
+ expect(caughtErr).not.toBe(undefined);
1310
+ expect(normalizeCodeLocInfo(caughtErr.message)).toContain(
1311
'The `style` prop expects a mapping from style properties to values, ' +
1312
"not a string. For example, style={{marginRight: spacing + 'em'}} " +
1311
- 'when using JSX.\n\nThis DOM node was rendered by `Animal`.',
1313
+ 'when using JSX.' +
1314
+ '\n in div (at **)' +
1315
+ '\n in Animal (at **)',
1316
);
1317
});
1318
packages/react-dom/src/shared/__tests__/ReactServerRendering-test.js
+26
-4
@@ -18,6 +18,10 @@ var PropTypes;
18
19
var ROOT_ATTRIBUTE_NAME;
20
21
+function normalizeCodeLocInfo(str) {
22
+ return str && str.replace(/\(at .+?:\d+\)/g, '(at **)');
23
+}
24
+
25
describe('ReactDOMServer', () => {
26
beforeEach(() => {
27
jest.resetModules();
@@ -380,11 +384,17 @@ describe('ReactDOMServer', () => {
384
});
385
386
it('should throw prop mapping error for an <iframe /> with invalid props', () => {
383
- expect(() =>
384
- ReactDOMServer.renderToString(<iframe style="border:none;" />),
385
- ).toThrowError(
387
+ let caughtErr;
388
+ try {
389
+ ReactDOMServer.renderToString(<iframe style="border:none;" />);
390
+ } catch (err) {
391
+ caughtErr = err;
392
+ }
393
+ expect(caughtErr).not.toBe(undefined);
394
+ expect(normalizeCodeLocInfo(caughtErr.message)).toContain(
395
'The `style` prop expects a mapping from style properties to values, not ' +
387
- "a string. For example, style={{marginRight: spacing + 'em'}} when using JSX.",
396
+ "a string. For example, style={{marginRight: spacing + 'em'}} when using JSX." +
397
+ '\n in iframe (at **)',
398
);
399
});
400
});
@@ -724,4 +734,16 @@ describe('ReactDOMServer', () => {
734
'HTML tags in React.',
735
);
736
});
737
+
738
+ it('should warn about contentEditable and children', () => {
739
+ spyOn(console, 'error');
740
+ ReactDOMServer.renderToString(<div contentEditable={true} children="" />);
741
+ expectDev(console.error.calls.count()).toBe(1);
742
+ expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
743
+ 'Warning: A component is `contentEditable` and contains `children` ' +
744
+ 'managed by React. It is now your responsibility to guarantee that ' +
745
+ 'none of those nodes are unexpectedly modified or duplicated. This ' +
746
+ 'is probably not intentional.\n in div (at **)',
747
+ );
748
+ });
749
});
packages/react-dom/src/shared/utils/assertValidProps.js
+4
-20
@@ -13,28 +13,12 @@ var invariant = require('fbjs/lib/invariant');
13
var voidElementTags = require('voidElementTags');
14
15
if (__DEV__) {
16
- var {getCurrentFiberStackAddendum} = require('ReactDebugCurrentFiber');
16
var warning = require('fbjs/lib/warning');
17
}
18
19
var HTML = '__html';
20
22
-function getDeclarationErrorAddendum(getCurrentOwnerName) {
23
- if (__DEV__) {
24
- var ownerName = getCurrentOwnerName();
25
- if (ownerName) {
26
- // TODO: also report the stack.
27
- return '\n\nThis DOM node was rendered by `' + ownerName + '`.';
28
- }
29
- }
30
- return '';
31
-}
32
-
33
-function assertValidProps(
34
- tag: string,
35
- props: ?Object,
36
- getCurrentOwnerName: () => ?string,
37
-) {
21
+function assertValidProps(tag: string, props: ?Object, getStack: () => string) {
22
if (!props) {
23
return;
24
}
@@ -45,7 +29,7 @@ function assertValidProps(
29
'%s is a void element tag and must neither have `children` nor ' +
30
'use `dangerouslySetInnerHTML`.%s',
31
tag,
48
- getDeclarationErrorAddendum(getCurrentOwnerName),
32
+ getStack(),
33
);
34
}
35
if (props.dangerouslySetInnerHTML != null) {
@@ -70,7 +54,7 @@ function assertValidProps(
54
'React. It is now your responsibility to guarantee that none of ' +
55
'those nodes are unexpectedly modified or duplicated. This is ' +
56
'probably not intentional.%s',
73
- getCurrentFiberStackAddendum() || '',
57
+ getStack(),
58
);
59
}
60
invariant(
@@ -78,7 +62,7 @@ function assertValidProps(
62
'The `style` prop expects a mapping from style properties to values, ' +
63
"not a string. For example, style={{marginRight: spacing + 'em'}} when " +
64
'using JSX.%s',
81
- getDeclarationErrorAddendum(getCurrentOwnerName),
65
+ getStack(),
66
);
67
}
68
scripts/rollup/bundles.js
+2
-12
@@ -169,12 +169,7 @@ const bundles = [
169
label: 'dom-server-browser',
170
manglePropertiesOnProd: false,
171
name: 'react-dom/server.browser',
172
- paths: [
173
- 'packages/react-dom/**/*.js',
174
- // TODO: server shouldn't depend on reconciler modules:
175
- 'packages/react-reconciler/**/*.js',
176
- 'packages/shared/**/*.js',
177
- ],
172
+ paths: ['packages/react-dom/**/*.js', 'packages/shared/**/*.js'],
173
},
174
175
{
@@ -194,12 +189,7 @@ const bundles = [
189
label: 'dom-server-server-node',
190
manglePropertiesOnProd: false,
191
name: 'react-dom/server.node',
197
- paths: [
198
- 'packages/react-dom/**/*.js',
199
- // TODO: server shouldn't depend on reconciler modules:
200
- 'packages/react-reconciler/**/*.js',
201
- 'packages/shared/**/*.js',
202
- ],
192
+ paths: ['packages/react-dom/**/*.js', 'packages/shared/**/*.js'],
193
},
194
195
/******* React ART *******/