Skip to content

Commit 47e49ae

Browse files
keyzkeyz
committed
Add component stack info to key validation warnings (#6799)
* Add component stack info to key validation warnings * Add `ReactComponentTreeDevtool.getStackAddendumByID`
1 parent 6a3e9d5 commit 47e49ae

11 files changed

Lines changed: 375 additions & 101 deletions

File tree

‎src/addons/transitions/ReactTransitionChildMapping.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,18 @@ var ReactTransitionChildMapping = {
1919
* simple syntactic sugar around flattenChildren().
2020
*
2121
* @param {*} children `this.props.children`
22+
* @param {number=} selfDebugID Optional debugID of the current internal instance.
2223
* @return {object} Mapping of key to child
2324
*/
24-
getChildMapping: function(children) {
25+
getChildMapping: function(children, selfDebugID) {
2526
if (!children) {
2627
return children;
2728
}
29+
30+
if (__DEV__) {
31+
return flattenChildren(children, selfDebugID);
32+
}
33+
2834
return flattenChildren(children);
2935
},
3036

‎src/addons/transitions/ReactTransitionGroup.js‎

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
'use strict';
1313

1414
var React = require('React');
15+
var ReactInstanceMap = require('ReactInstanceMap');
1516
var ReactTransitionChildMapping = require('ReactTransitionChildMapping');
1617

1718
var emptyFunction = require('emptyFunction');
@@ -38,6 +39,7 @@ var ReactTransitionGroup = React.createClass({
3839

3940
getInitialState: function() {
4041
return {
42+
// TODO: can we get useful debug information to show at this point?
4143
children: ReactTransitionChildMapping.getChildMapping(this.props.children),
4244
};
4345
},
@@ -58,9 +60,17 @@ var ReactTransitionGroup = React.createClass({
5860
},
5961

6062
componentWillReceiveProps: function(nextProps) {
61-
var nextChildMapping = ReactTransitionChildMapping.getChildMapping(
62-
nextProps.children
63-
);
63+
var nextChildMapping;
64+
if (__DEV__) {
65+
nextChildMapping = ReactTransitionChildMapping.getChildMapping(
66+
nextProps.children,
67+
ReactInstanceMap.get(this)._debugID
68+
);
69+
} else {
70+
nextChildMapping = ReactTransitionChildMapping.getChildMapping(
71+
nextProps.children
72+
);
73+
}
6474
var prevChildMapping = this.state.children;
6575

6676
this.setState({
@@ -123,9 +133,17 @@ var ReactTransitionGroup = React.createClass({
123133

124134
delete this.currentlyTransitioningKeys[key];
125135

126-
var currentChildMapping = ReactTransitionChildMapping.getChildMapping(
127-
this.props.children
128-
);
136+
var currentChildMapping;
137+
if (__DEV__) {
138+
currentChildMapping = ReactTransitionChildMapping.getChildMapping(
139+
this.props.children,
140+
ReactInstanceMap.get(this)._debugID
141+
);
142+
} else {
143+
currentChildMapping = ReactTransitionChildMapping.getChildMapping(
144+
this.props.children
145+
);
146+
}
129147

130148
if (!currentChildMapping || !currentChildMapping.hasOwnProperty(key)) {
131149
// This was removed before it had fully appeared. Remove it.
@@ -155,9 +173,17 @@ var ReactTransitionGroup = React.createClass({
155173

156174
delete this.currentlyTransitioningKeys[key];
157175

158-
var currentChildMapping = ReactTransitionChildMapping.getChildMapping(
159-
this.props.children
160-
);
176+
var currentChildMapping;
177+
if (__DEV__) {
178+
currentChildMapping = ReactTransitionChildMapping.getChildMapping(
179+
this.props.children,
180+
ReactInstanceMap.get(this)._debugID
181+
);
182+
} else {
183+
currentChildMapping = ReactTransitionChildMapping.getChildMapping(
184+
this.props.children
185+
);
186+
}
161187

162188
if (!currentChildMapping || !currentChildMapping.hasOwnProperty(key)) {
163189
// This was removed before it had fully entered. Remove it.
@@ -188,9 +214,17 @@ var ReactTransitionGroup = React.createClass({
188214

189215
delete this.currentlyTransitioningKeys[key];
190216

191-
var currentChildMapping = ReactTransitionChildMapping.getChildMapping(
192-
this.props.children
193-
);
217+
var currentChildMapping;
218+
if (__DEV__) {
219+
currentChildMapping = ReactTransitionChildMapping.getChildMapping(
220+
this.props.children,
221+
ReactInstanceMap.get(this)._debugID
222+
);
223+
} else {
224+
currentChildMapping = ReactTransitionChildMapping.getChildMapping(
225+
this.props.children
226+
);
227+
}
194228

195229
if (currentChildMapping && currentChildMapping.hasOwnProperty(key)) {
196230
// This entered again before it fully left. Add it again.

‎src/addons/transitions/__tests__/ReactTransitionGroup-test.js‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ var ReactTransitionGroup;
2020
describe('ReactTransitionGroup', function() {
2121
var container;
2222

23+
function normalizeCodeLocInfo(str) {
24+
return str.replace(/\(at .+?:\d+\)/g, '(at **)');
25+
}
26+
2327
beforeEach(function() {
2428
React = require('React');
2529
ReactDOM = require('ReactDOM');
@@ -269,4 +273,33 @@ describe('ReactTransitionGroup', function() {
269273
'willLeave2', 'didLeave2', 'willUnmount0', 'willUnmount1', 'willUnmount2',
270274
]);
271275
});
276+
277+
it('should warn for duplicated keys with component stack info', function() {
278+
spyOn(console, 'error');
279+
280+
var Component = React.createClass({
281+
render: function() {
282+
var children = [<div key="1"/>, <div key="1" />];
283+
return <ReactTransitionGroup>{children}</ReactTransitionGroup>;
284+
},
285+
});
286+
287+
ReactDOM.render(<Component />, container);
288+
289+
expect(console.error.argsForCall.length).toBe(2);
290+
expect(console.error.argsForCall[0][0]).toBe(
291+
'Warning: flattenChildren(...): ' +
292+
'Encountered two children with the same key, `1`. ' +
293+
'Child keys must be unique; when two children share a key, ' +
294+
'only the first child will be used.'
295+
);
296+
expect(normalizeCodeLocInfo(console.error.argsForCall[1][0])).toBe(
297+
'Warning: flattenChildren(...): ' +
298+
'Encountered two children with the same key, `1`. ' +
299+
'Child keys must be unique; when two children share a key, ' +
300+
'only the first child will be used.\n' +
301+
' in ReactTransitionGroup (at **)\n' +
302+
' in Component (at **)'
303+
);
304+
});
272305
});

‎src/isomorphic/classic/element/ReactElementValidator.js‎

Lines changed: 31 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,25 @@ var ownerHasKeyUseWarning = {};
4848

4949
var loggedTypeFailures = {};
5050

51+
function getCurrentComponentErrorInfo(parentType) {
52+
var info = getDeclarationErrorAddendum();
53+
54+
if (!info) {
55+
var parentName = typeof parentType === 'string' ?
56+
parentType : parentType.displayName || parentType.name;
57+
if (parentName) {
58+
info = ` Check the top-level render call using <${parentName}>.`;
59+
}
60+
}
61+
return info;
62+
}
63+
5164
/**
5265
* Warn if the element doesn't have an explicit key assigned to it.
5366
* This element is in an array. The array could grow and shrink or be
5467
* reordered. All children that haven't already been validated are required to
55-
* have a "key" property assigned to it.
68+
* have a "key" property assigned to it. Error statuses are cached so a warning
69+
* will only be shown once.
5670
*
5771
* @internal
5872
* @param {ReactElement} element Element that requires a key.
@@ -64,67 +78,36 @@ function validateExplicitKey(element, parentType) {
6478
}
6579
element._store.validated = true;
6680

67-
var addenda = getAddendaForKeyUse('uniqueKey', element, parentType);
68-
if (addenda === null) {
69-
// we already showed the warning
70-
return;
71-
}
72-
warning(
73-
false,
74-
'Each child in an array or iterator should have a unique "key" prop.' +
75-
'%s%s%s',
76-
addenda.parentOrOwner || '',
77-
addenda.childOwner || '',
78-
addenda.url || ''
81+
var memoizer = ownerHasKeyUseWarning.uniqueKey || (
82+
ownerHasKeyUseWarning.uniqueKey = {}
7983
);
80-
}
81-
82-
/**
83-
* Shared warning and monitoring code for the key warnings.
84-
*
85-
* @internal
86-
* @param {string} messageType A key used for de-duping warnings.
87-
* @param {ReactElement} element Component that requires a key.
88-
* @param {*} parentType element's parent's type.
89-
* @returns {?object} A set of addenda to use in the warning message, or null
90-
* if the warning has already been shown before (and shouldn't be shown again).
91-
*/
92-
function getAddendaForKeyUse(messageType, element, parentType) {
93-
var addendum = getDeclarationErrorAddendum();
94-
if (!addendum) {
95-
var parentName = typeof parentType === 'string' ?
96-
parentType : parentType.displayName || parentType.name;
97-
if (parentName) {
98-
addendum = ` Check the top-level render call using <${parentName}>.`;
99-
}
100-
}
10184

102-
var memoizer = ownerHasKeyUseWarning[messageType] || (
103-
ownerHasKeyUseWarning[messageType] = {}
104-
);
105-
if (memoizer[addendum]) {
106-
return null;
85+
var currentComponentErrorInfo = getCurrentComponentErrorInfo(parentType);
86+
if (memoizer[currentComponentErrorInfo]) {
87+
return;
10788
}
108-
memoizer[addendum] = true;
109-
110-
var addenda = {
111-
parentOrOwner: addendum,
112-
url: ' See https://fb.me/react-warning-keys for more information.',
113-
childOwner: null,
114-
};
89+
memoizer[currentComponentErrorInfo] = true;
11590

11691
// Usually the current owner is the offender, but if it accepts children as a
11792
// property, it may be the creator of the child that's responsible for
11893
// assigning it a key.
94+
var childOwner = '';
11995
if (element &&
12096
element._owner &&
12197
element._owner !== ReactCurrentOwner.current) {
12298
// Give the component that originally created this child.
123-
addenda.childOwner =
99+
childOwner =
124100
` It was passed a child from ${element._owner.getName()}.`;
125101
}
126102

127-
return addenda;
103+
warning(
104+
false,
105+
'Each child in an array or iterator should have a unique "key" prop.' +
106+
'%s%s See https://fb.me/react-warning-keys for more information.%s',
107+
currentComponentErrorInfo,
108+
childOwner,
109+
ReactComponentTreeDevtool.getCurrentStackAddendum(element)
110+
);
128111
}
129112

130113
/**

‎src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ var ReactDOM;
1919
var ReactTestUtils;
2020

2121
describe('ReactElementValidator', function() {
22+
function normalizeCodeLocInfo(str) {
23+
return str.replace(/\(at .+?:\d+\)/g, '(at **)');
24+
}
25+
2226
var ComponentClass;
2327

2428
beforeEach(function() {
@@ -95,9 +99,10 @@ describe('ReactElementValidator', function() {
9599
ReactTestUtils.renderIntoDocument(<Anonymous>{divs}</Anonymous>);
96100

97101
expect(console.error.argsForCall.length).toBe(1);
98-
expect(console.error.argsForCall[0][0]).toBe(
102+
expect(normalizeCodeLocInfo(console.error.argsForCall[0][0])).toBe(
99103
'Warning: Each child in an array or iterator should have a unique ' +
100-
'"key" prop. See https://fb.me/react-warning-keys for more information.'
104+
'"key" prop. See https://fb.me/react-warning-keys for more information.\n' +
105+
' in div (at **)'
101106
);
102107
});
103108

@@ -111,10 +116,46 @@ describe('ReactElementValidator', function() {
111116
ReactTestUtils.renderIntoDocument(<div>{divs}</div>);
112117

113118
expect(console.error.argsForCall.length).toBe(1);
114-
expect(console.error.argsForCall[0][0]).toBe(
119+
expect(normalizeCodeLocInfo(console.error.argsForCall[0][0])).toBe(
115120
'Warning: Each child in an array or iterator should have a unique ' +
116121
'"key" prop. Check the top-level render call using <div>. See ' +
117-
'https://fb.me/react-warning-keys for more information.'
122+
'https://fb.me/react-warning-keys for more information.\n' +
123+
' in div (at **)'
124+
);
125+
});
126+
127+
it('warns for keys with component stack info', function() {
128+
spyOn(console, 'error');
129+
130+
var Component = React.createClass({
131+
render: function() {
132+
return <div>{[<div />, <div />]}</div>;
133+
},
134+
});
135+
136+
var Parent = React.createClass({
137+
render: function() {
138+
return React.cloneElement(this.props.child);
139+
},
140+
});
141+
142+
var GrandParent = React.createClass({
143+
render: function() {
144+
return <Parent child={<Component />} />;
145+
},
146+
});
147+
148+
ReactTestUtils.renderIntoDocument(<GrandParent />);
149+
150+
expect(console.error.argsForCall.length).toBe(1);
151+
expect(normalizeCodeLocInfo(console.error.argsForCall[0][0])).toBe(
152+
'Warning: Each child in an array or iterator should have a unique ' +
153+
'"key" prop. Check the render method of `Component`. See ' +
154+
'https://fb.me/react-warning-keys for more information.\n' +
155+
' in div (at **)\n' +
156+
' in Component (at **)\n' +
157+
' in Parent (at **)\n' +
158+
' in GrandParent (at **)'
118159
);
119160
});
120161

0 commit comments

Comments
 (0)