Skip to content

Commit 1a81be4

Browse files
gaearongaearon
authored
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
1 parent f56ca47 commit 1a81be4

File tree

6 files changed

+88
-92
lines changed

6 files changed

+88
-92
lines changed

‎packages/react-dom/src/ReactDOMFiberComponent.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ var HTML = '__html';
6161

6262
var {Namespaces: {html: HTML_NAMESPACE}, getIntrinsicNamespace} = DOMNamespaces;
6363

64-
var getStack = emptyFunction;
64+
var getStack = emptyFunction.thatReturnsArgument('');
6565

6666
if (__DEV__) {
6767
getStack = getCurrentFiberStackAddendum;
@@ -562,7 +562,7 @@ var ReactDOMFiberComponent = {
562562
props = rawProps;
563563
}
564564

565-
assertValidProps(tag, props, getCurrentFiberOwnerName);
565+
assertValidProps(tag, props, getStack);
566566

567567
setInitialDOMProperties(
568568
tag,
@@ -656,7 +656,7 @@ var ReactDOMFiberComponent = {
656656
break;
657657
}
658658

659-
assertValidProps(tag, nextProps, getCurrentFiberOwnerName);
659+
assertValidProps(tag, nextProps, getStack);
660660

661661
var propKey;
662662
var styleName;
@@ -971,7 +971,7 @@ var ReactDOMFiberComponent = {
971971
break;
972972
}
973973

974-
assertValidProps(tag, rawProps, getCurrentFiberOwnerName);
974+
assertValidProps(tag, rawProps, getStack);
975975

976976
if (__DEV__) {
977977
var extraAttributeNames: Set<string> = new Set();

‎packages/react-dom/src/server/ReactPartialRenderer.js

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ var omittedCloseTags = require('omittedCloseTags');
3030
var isCustomComponent = require('isCustomComponent');
3131

3232
var toArray = React.Children.toArray;
33-
var emptyFunctionThatReturnsNull = emptyFunction.thatReturnsNull;
33+
var getStackAddendum = emptyFunction.thatReturnsArgument('');
3434

3535
if (__DEV__) {
3636
var warning = require('fbjs/lib/warning');
@@ -80,9 +80,9 @@ if (__DEV__) {
8080
currentDebugStack = null;
8181
ReactDebugCurrentFrame.getCurrentStack = null;
8282
};
83-
var getStackAddendum = function(): null | string {
83+
getStackAddendum = function(): null | string {
8484
if (currentDebugStack === null) {
85-
return null;
85+
return '';
8686
}
8787
let stack = '';
8888
let debugStack = currentDebugStack;
@@ -141,11 +141,7 @@ function createMarkupForStyles(styles) {
141141
var styleValue = styles[styleName];
142142
if (__DEV__) {
143143
if (!isCustomProperty) {
144-
warnValidStyle(
145-
styleName,
146-
styleValue,
147-
() => '', // getCurrentFiberStackAddendum,
148-
);
144+
warnValidStyle(styleName, styleValue, getStackAddendum);
149145
}
150146
}
151147
if (styleValue != null) {
@@ -574,7 +570,7 @@ class ReactDOMServerRenderer {
574570
ReactControlledValuePropTypes.checkPropTypes(
575571
'input',
576572
props,
577-
() => '', //getCurrentFiberStackAddendum
573+
getStackAddendum,
578574
);
579575

580576
if (
@@ -632,7 +628,7 @@ class ReactDOMServerRenderer {
632628
ReactControlledValuePropTypes.checkPropTypes(
633629
'textarea',
634630
props,
635-
() => '', //getCurrentFiberStackAddendum
631+
getStackAddendum,
636632
);
637633
if (
638634
props.value !== undefined &&
@@ -693,7 +689,7 @@ class ReactDOMServerRenderer {
693689
ReactControlledValuePropTypes.checkPropTypes(
694690
'select',
695691
props,
696-
() => '', // getCurrentFiberStackAddendum,
692+
getStackAddendum,
697693
);
698694

699695
for (var i = 0; i < valuePropNames.length; i++) {
@@ -785,7 +781,7 @@ class ReactDOMServerRenderer {
785781
validatePropertiesInDevelopment(tag, props);
786782
}
787783

788-
assertValidProps(tag, props, emptyFunctionThatReturnsNull);
784+
assertValidProps(tag, props, getStackAddendum);
789785

790786
var out = createOpenTagMarkup(
791787
element.type,

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

Lines changed: 44 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -954,50 +954,38 @@ describe('ReactDOMComponent', () => {
954954
});
955955

956956
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 {
960960
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(
962966
'input is a void element tag and must neither have `children` nor ' +
963967
'use `dangerouslySetInnerHTML`.',
968+
'\n in input (at **)',
964969
);
965970
});
966971

967972
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 {
971976
ReactDOM.render(
972977
<input dangerouslySetInnerHTML={{__html: 'content'}} />,
973978
container,
974979
);
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;
996982
}
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+
);
1001989
});
1002990

1003991
it('should emit a warning once for a named custom component using shady DOM', () => {
@@ -1178,19 +1166,27 @@ describe('ReactDOMComponent', () => {
11781166
expect(tracker.getValue()).toEqual('foo');
11791167
});
11801168

1181-
it('should warn for children on void elements', () => {
1169+
it('should throw for children on void elements', () => {
11821170
class X extends React.Component {
11831171
render() {
11841172
return <input>moo</input>;
11851173
}
11861174
}
11871175

1188-
var container = document.createElement('div');
1189-
expect(function() {
1176+
const container = document.createElement('div');
1177+
let caughtErr;
1178+
try {
11901179
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(
11921186
'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 **)',
11941190
);
11951191
});
11961192

@@ -1303,12 +1299,20 @@ describe('ReactDOMComponent', () => {
13031299
}
13041300
}
13051301

1306-
expect(function() {
1302+
let caughtErr;
1303+
try {
13071304
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(
13091311
'The `style` prop expects a mapping from style properties to values, ' +
13101312
"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 **)',
13121316
);
13131317
});
13141318

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

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ var PropTypes;
1818

1919
var ROOT_ATTRIBUTE_NAME;
2020

21+
function normalizeCodeLocInfo(str) {
22+
return str && str.replace(/\(at .+?:\d+\)/g, '(at **)');
23+
}
24+
2125
describe('ReactDOMServer', () => {
2226
beforeEach(() => {
2327
jest.resetModules();
@@ -380,11 +384,17 @@ describe('ReactDOMServer', () => {
380384
});
381385

382386
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(
386395
'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 **)',
388398
);
389399
});
390400
});
@@ -724,4 +734,16 @@ describe('ReactDOMServer', () => {
724734
'HTML tags in React.',
725735
);
726736
});
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+
});
727749
});

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

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,28 +13,12 @@ var invariant = require('fbjs/lib/invariant');
1313
var voidElementTags = require('voidElementTags');
1414

1515
if (__DEV__) {
16-
var {getCurrentFiberStackAddendum} = require('ReactDebugCurrentFiber');
1716
var warning = require('fbjs/lib/warning');
1817
}
1918

2019
var HTML = '__html';
2120

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) {
3822
if (!props) {
3923
return;
4024
}
@@ -45,7 +29,7 @@ function assertValidProps(
4529
'%s is a void element tag and must neither have `children` nor ' +
4630
'use `dangerouslySetInnerHTML`.%s',
4731
tag,
48-
getDeclarationErrorAddendum(getCurrentOwnerName),
32+
getStack(),
4933
);
5034
}
5135
if (props.dangerouslySetInnerHTML != null) {
@@ -70,15 +54,15 @@ function assertValidProps(
7054
'React. It is now your responsibility to guarantee that none of ' +
7155
'those nodes are unexpectedly modified or duplicated. This is ' +
7256
'probably not intentional.%s',
73-
getCurrentFiberStackAddendum() || '',
57+
getStack(),
7458
);
7559
}
7660
invariant(
7761
props.style == null || typeof props.style === 'object',
7862
'The `style` prop expects a mapping from style properties to values, ' +
7963
"not a string. For example, style={{marginRight: spacing + 'em'}} when " +
8064
'using JSX.%s',
81-
getDeclarationErrorAddendum(getCurrentOwnerName),
65+
getStack(),
8266
);
8367
}
8468

‎scripts/rollup/bundles.js

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -169,12 +169,7 @@ const bundles = [
169169
label: 'dom-server-browser',
170170
manglePropertiesOnProd: false,
171171
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'],
178173
},
179174

180175
{
@@ -194,12 +189,7 @@ const bundles = [
194189
label: 'dom-server-server-node',
195190
manglePropertiesOnProd: false,
196191
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'],
203193
},
204194

205195
/******* React ART *******/

0 commit comments

Comments
 (0)