Skip to content

Commit 39402fe

Browse files
committed
Revert "Add key warning to nested collections"
This heuristic isn't great because it relies on inspecting deep children which aren't guaranteed to be React elements. In particular, this was causing stack overflows in a component we had that used a *DOM node* as children, like `<DOMContainer>{node}</DOMContainer>`. This reverts commits: 0a3aa84 64c9d9d 0c58f4f 8cf226e 0866367
1 parent 6502da8 commit 39402fe

File tree

4 files changed

+37
-111
lines changed

4 files changed

+37
-111
lines changed

src/addons/__tests__/ReactFragment-test.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,13 @@ describe('ReactFragment', function() {
4949
z: <span />
5050
};
5151
var element = <div>{[children]}</div>;
52+
expect(console.error.calls.length).toBe(0);
53+
var container = document.createElement('div');
54+
React.render(element, container);
5255
expect(console.error.calls.length).toBe(1);
5356
expect(console.error.calls[0].args[0]).toContain(
5457
'Any use of a keyed object'
5558
);
56-
var container = document.createElement('div');
57-
React.render(element, container);
58-
expect(console.error.calls.length).toBe(1);
5959
});
6060

6161
it('should warn if accessing any property on a fragment', function() {

src/isomorphic/classic/element/ReactElement.js

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -80,21 +80,6 @@ function defineMutationMembrane(prototype) {
8080
}
8181
}
8282

83-
function markChildArrayValidated(childArray) {
84-
// To make comparing ReactElements easier for testing purposes, we make the
85-
// validation flag non-enumerable (where possible, which should include every
86-
// environment we run tests in), so the test framework ignores it.
87-
try {
88-
Object.defineProperty(childArray, '_reactChildKeysValidated', {
89-
configurable: false,
90-
enumerable: false,
91-
writable: true
92-
});
93-
} catch (x) {
94-
}
95-
childArray._reactChildKeysValidated = true;
96-
}
97-
9883
/**
9984
* Base constructor for all React elements. This is only used to make this
10085
* work with a dynamic instanceof check. Nothing should live on this prototype.
@@ -121,6 +106,20 @@ var ReactElement = function(type, key, ref, owner, context, props) {
121106
// commonly used development environments.
122107
this._store = {props: props, originalProps: assign({}, props)};
123108

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+
124123
// We're not allowed to set props directly on the object so we early
125124
// return and rely on the prototype membrane to forward to the backing
126125
// store.
@@ -171,9 +170,6 @@ ReactElement.createElement = function(type, config, children) {
171170
props.children = children;
172171
} else if (childrenLength > 1) {
173172
var childArray = Array(childrenLength);
174-
if (__DEV__) {
175-
markChildArrayValidated(childArray);
176-
}
177173
for (var i = 0; i < childrenLength; i++) {
178174
childArray[i] = arguments[i + 2];
179175
}
@@ -220,6 +216,11 @@ ReactElement.cloneAndReplaceProps = function(oldElement, newProps) {
220216
oldElement._context,
221217
newProps
222218
);
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+
}
223224
return newElement;
224225
};
225226

@@ -261,9 +262,6 @@ ReactElement.cloneElement = function(element, config, children) {
261262
props.children = children;
262263
} else if (childrenLength > 1) {
263264
var childArray = Array(childrenLength);
264-
if (__DEV__) {
265-
markChildArrayValidated(childArray);
266-
}
267265
for (var i = 0; i < childrenLength; i++) {
268266
childArray[i] = arguments[i + 2];
269267
}

src/isomorphic/classic/element/ReactElementValidator.js

Lines changed: 15 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -90,19 +90,15 @@ function getCurrentOwnerDisplayName() {
9090
* @internal
9191
* @param {ReactElement} element Element that requires a key.
9292
* @param {*} parentType element's parent's type.
93-
* @param {boolean} deep false if top-level collection, true if nested
9493
*/
95-
function validateExplicitKey(element, parentType, deep) {
96-
if (element.key != null) {
94+
function validateExplicitKey(element, parentType) {
95+
if (element._store.validated || element.key != null) {
9796
return;
9897
}
98+
element._store.validated = true;
99+
99100
warnAndMonitorForKeyUse(
100-
// We vary the message for nested key warning to allow filtering them out
101-
// since we didn't historically warn in this case.
102-
deep ?
103-
'Each child in a nested array or iterator should have ' +
104-
'a unique "key" prop.' :
105-
'Each child in an array or iterator should have a unique "key" prop.',
101+
'Each child in an array or iterator should have a unique "key" prop.',
106102
element,
107103
parentType
108104
);
@@ -116,17 +112,13 @@ function validateExplicitKey(element, parentType, deep) {
116112
* @param {string} name Property name of the key.
117113
* @param {ReactElement} element Component that requires a key.
118114
* @param {*} parentType element's parent's type.
119-
* @param {boolean} deep false if top-level collection, true if nested
120115
*/
121-
function validatePropertyKey(name, element, parentType, deep) {
116+
function validatePropertyKey(name, element, parentType) {
122117
if (!NUMERIC_PROPERTY_REGEX.test(name)) {
123118
return;
124119
}
125120
warnAndMonitorForKeyUse(
126-
deep ?
127-
'Nested child objects should have non-numeric keys ' +
128-
'so ordering is preserved.' :
129-
'Child objects should have non-numeric keys so ordering is preserved.',
121+
'Child objects should have non-numeric keys so ordering is preserved.',
130122
element,
131123
parentType
132124
);
@@ -188,29 +180,18 @@ function warnAndMonitorForKeyUse(message, element, parentType) {
188180
* @internal
189181
* @param {ReactNode} node Statically passed child of any type.
190182
* @param {*} parentType node's parent's type.
191-
* @param {boolean} deep false if top-level collection, true if nested
192183
*/
193-
function validateChildKeys(node, parentType, deep) {
184+
function validateChildKeys(node, parentType) {
194185
if (Array.isArray(node)) {
195-
if (node._reactChildKeysValidated) {
196-
// All child elements were passed in a valid location.
197-
return;
198-
}
199186
for (var i = 0; i < node.length; i++) {
200187
var child = node[i];
201188
if (ReactElement.isValidElement(child)) {
202-
validateExplicitKey(child, parentType, deep);
203-
} else {
204-
// TODO: Warn on unkeyed arrays and suggest using createFragment
205-
validateChildKeys(child, parentType, true);
189+
validateExplicitKey(child, parentType);
206190
}
207191
}
208-
} else if (
209-
typeof node === 'string' || typeof node === 'number' ||
210-
ReactElement.isValidElement(node)
211-
) {
192+
} else if (ReactElement.isValidElement(node)) {
212193
// This element was passed in a valid location.
213-
return;
194+
node._store.validated = true;
214195
} else if (node) {
215196
var iteratorFn = getIteratorFn(node);
216197
// Entry iterators provide implicit keys.
@@ -220,18 +201,15 @@ function validateChildKeys(node, parentType, deep) {
220201
var step;
221202
while (!(step = iterator.next()).done) {
222203
if (ReactElement.isValidElement(step.value)) {
223-
validateExplicitKey(step.value, parentType, deep);
224-
} else {
225-
validateChildKeys(step.value, parentType, true);
204+
validateExplicitKey(step.value, parentType);
226205
}
227206
}
228207
}
229208
} else if (typeof node === 'object') {
230209
var fragment = ReactFragment.extractIfFragment(node);
231210
for (var key in fragment) {
232211
if (fragment.hasOwnProperty(key)) {
233-
validatePropertyKey(key, fragment[key], parentType, deep);
234-
validateChildKeys(fragment[key], parentType, true);
212+
validatePropertyKey(key, fragment[key], parentType);
235213
}
236214
}
237215
}
@@ -437,7 +415,7 @@ var ReactElementValidator = {
437415
}
438416

439417
for (var i = 2; i < arguments.length; i++) {
440-
validateChildKeys(arguments[i], type, false);
418+
validateChildKeys(arguments[i], type);
441419
}
442420

443421
validatePropTypes(element);
@@ -485,7 +463,7 @@ var ReactElementValidator = {
485463
cloneElement: function(element, props, children) {
486464
var newElement = ReactElement.cloneElement.apply(this, arguments);
487465
for (var i = 2; i < arguments.length; i++) {
488-
validateChildKeys(arguments[i], newElement.type, false);
466+
validateChildKeys(arguments[i], newElement.type);
489467
}
490468
validatePropTypes(newElement);
491469
return newElement;

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

Lines changed: 0 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -120,56 +120,6 @@ describe('ReactElementValidator', function() {
120120
);
121121
});
122122

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 a nested array or iterator should have a ' +
138-
'unique "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-
ReactTestUtils.renderIntoDocument(
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-
173123
it('does not warn for keys when passing children down', function() {
174124
spyOn(console, 'error');
175125

0 commit comments

Comments
 (0)