Add key warning to nested collections
Also when reusing elements in multiple contexts -- before we were mutating each element to indicate its validity; now we mutate the array containing it (which we create, in the case of rest-arg children). Fixes #2496. Fixes #3348.
Ben Alpert committed
Apr 27, 2015 at 14:16 UTC
086636747f26b577b4a4577a0888118310ee91b3
4 files changed
+105
-26
src/addons/__tests__/ReactFragment-test.js
+3
-3
@@ -49,13 +49,13 @@ describe('ReactFragment', function() {
49
z: <span />
50
};
51
var element = <div>{[children]}</div>;
52
- expect(console.error.calls.length).toBe(0);
53
- var container = document.createElement('div');
54
- React.render(element, container);
52
expect(console.error.calls.length).toBe(1);
53
expect(console.error.calls[0].args[0]).toContain(
54
'Any use of a keyed object'
55
);
56
+ var container = document.createElement('div');
57
+ React.render(element, container);
58
+ expect(console.error.calls.length).toBe(1);
59
});
60
61
it('should warn if accessing any property on a fragment', function() {
src/classic/element/ReactElement.js
+15
-19
@@ -106,20 +106,6 @@ var ReactElement = function(type, key, ref, owner, context, props) {
106
// commonly used development environments.
107
this._store = {props: props, originalProps: assign({}, props)};
108
109
- // To make comparing ReactElements easier for testing purposes, we make
110
- // the validation flag non-enumerable (where possible, which should
111
- // include every environment we run tests in), so the test framework
112
- // ignores it.
113
- try {
114
- Object.defineProperty(this._store, 'validated', {
115
- configurable: false,
116
- enumerable: false,
117
- writable: true
118
- });
119
- } catch (x) {
120
- }
121
- this._store.validated = false;
122
-
109
// We're not allowed to set props directly on the object so we early
110
// return and rely on the prototype membrane to forward to the backing
111
// store.
@@ -170,6 +156,21 @@ ReactElement.createElement = function(type, config, children) {
156
props.children = children;
157
} else if (childrenLength > 1) {
158
var childArray = Array(childrenLength);
159
+
160
+ // To make comparing ReactElements easier for testing purposes, we make
161
+ // the validation flag non-enumerable (where possible, which should
162
+ // include every environment we run tests in), so the test framework
163
+ // ignores it.
164
+ try {
165
+ Object.defineProperty(childArray, '_reactChildKeysValidated', {
166
+ configurable: false,
167
+ enumerable: false,
168
+ writable: true
169
+ });
170
+ } catch (x) {
171
+ }
172
+ childArray._reactChildKeysValidated = true;
173
+
174
for (var i = 0; i < childrenLength; i++) {
175
childArray[i] = arguments[i + 2];
176
}
@@ -216,11 +217,6 @@ ReactElement.cloneAndReplaceProps = function(oldElement, newProps) {
217
oldElement._context,
218
newProps
219
);
219
-
220
- if (__DEV__) {
221
- // If the key on the original is valid, then the clone is valid
222
- newElement._store.validated = oldElement._store.validated;
223
- }
220
return newElement;
221
};
222
src/classic/element/ReactElementValidator.js
+12
-4
@@ -92,11 +92,9 @@ function getCurrentOwnerDisplayName() {
92
* @param {*} parentType element's parent's type.
93
*/
94
function validateExplicitKey(element, parentType) {
95
- if (element._store.validated || element.key != null) {
95
+ if (element.key != null) {
96
return;
97
}
98
- element._store.validated = true;
99
-
98
warnAndMonitorForKeyUse(
99
'Each child in an array or iterator should have a unique "key" prop.',
100
element,
@@ -183,15 +181,22 @@ function warnAndMonitorForKeyUse(message, element, parentType) {
181
*/
182
function validateChildKeys(node, parentType) {
183
if (Array.isArray(node)) {
184
+ if (node._reactChildKeysValidated) {
185
+ // All child elements were passed in a valid location.
186
+ return;
187
+ }
188
for (var i = 0; i < node.length; i++) {
189
var child = node[i];
190
if (ReactElement.isValidElement(child)) {
191
validateExplicitKey(child, parentType);
192
+ } else {
193
+ // TODO: Warn on unkeyed arrays and suggest using createFragment
194
+ validateChildKeys(child, parentType);
195
}
196
}
197
} else if (ReactElement.isValidElement(node)) {
198
// This element was passed in a valid location.
194
- node._store.validated = true;
199
+ return;
200
} else if (node) {
201
var iteratorFn = getIteratorFn(node);
202
// Entry iterators provide implicit keys.
@@ -202,6 +207,8 @@ function validateChildKeys(node, parentType) {
207
while (!(step = iterator.next()).done) {
208
if (ReactElement.isValidElement(step.value)) {
209
validateExplicitKey(step.value, parentType);
210
+ } else {
211
+ validateChildKeys(step.value, parentType);
212
}
213
}
214
}
@@ -210,6 +217,7 @@ function validateChildKeys(node, parentType) {
217
for (var key in fragment) {
218
if (fragment.hasOwnProperty(key)) {
219
validatePropertyKey(key, fragment[key], parentType);
220
+ validateChildKeys(fragment[key], parentType);
221
}
222
}
223
}
src/classic/element/__tests__/ReactElementValidator-test.js
+75
@@ -120,6 +120,81 @@ describe('ReactElementValidator', function() {
120
);
121
});
122
123
+ it('warns for keys for nested arrays of elements', function() {
124
+ spyOn(console, 'error');
125
+
126
+ var divs = [
127
+ [
128
+ <div />,
129
+ <div />
130
+ ],
131
+ <div key="foo" />
132
+ ];
133
+ ReactTestUtils.renderIntoDocument(<div>{divs}</div>);
134
+
135
+ expect(console.error.argsForCall.length).toBe(1);
136
+ expect(console.error.argsForCall[0][0]).toBe(
137
+ 'Warning: Each child in an array or iterator should have a unique ' +
138
+ '"key" prop. Check the React.render call using <div>. See ' +
139
+ 'https://fb.me/react-warning-keys for more information.'
140
+ );
141
+ });
142
+
143
+ it('warns for keys when reusing children', function() {
144
+ spyOn(console, 'error');
145
+
146
+ var f = <span />;
147
+ var g = <span />;
148
+
149
+ var children = [f, g];
150
+
151
+ return (
152
+ <div>
153
+ <div key="0">
154
+ {g}
155
+ </div>
156
+ <div key="1">
157
+ {f}
158
+ </div>
159
+ <div key="2">
160
+ {children}
161
+ </div>
162
+ </div>
163
+ );
164
+
165
+ expect(console.error.argsForCall.length).toBe(1);
166
+ expect(console.error.argsForCall[0][0]).toBe(
167
+ 'Warning: Each child in an array or iterator should have a unique ' +
168
+ '"key" prop. Check the React.render call using <div>. See ' +
169
+ 'https://fb.me/react-warning-keys for more information.'
170
+ );
171
+ });
172
+
173
+ it('does not warn for keys when passing children down', function() {
174
+ spyOn(console, 'error');
175
+
176
+ debugger;
177
+ var Wrapper = React.createClass({
178
+ render: function() {
179
+ return (
180
+ <div>
181
+ {this.props.children}
182
+ <footer />
183
+ </div>
184
+ );
185
+ }
186
+ });
187
+
188
+ ReactTestUtils.renderIntoDocument(
189
+ <Wrapper>
190
+ <span />
191
+ <span />
192
+ </Wrapper>
193
+ );
194
+
195
+ expect(console.error.argsForCall.length).toBe(0);
196
+ });
197
+
198
it('warns for keys for iterables of elements in rest args', function() {
199
spyOn(console, 'error');
200
var Component = React.createFactory(ComponentClass);