Facebook/react
1
0
Fork 0
mirror of https://github.com/facebook/react.git synced 2025-11-01 09:12:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3883 from spicyj/revert-nested

Browse Source 
Revert "Add key warning to nested collections"
This commit is contained in:
Ben Alpert
2015-05-18 23:11:29 -07:00
parent 6502da8f8d 69e6ab5083
commit e47fa115bb
4 changed files with 54 additions and 111 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/addons/__tests__/ReactFragment-test.js
+3 -3
View File
@@ -49,13 +49,13 @@ describe('ReactFragment', function() {
z: <span />
};
var element = <div>{[children]}</div>;
expect(console.error.calls.length).toBe(0);
var container = document.createElement('div');
React.render(element, container);
expect(console.error.calls.length).toBe(1);
expect(console.error.calls[0].args[0]).toContain(
'Any use of a keyed object'
);
var container = document.createElement('div');
React.render(element, container);
expect(console.error.calls.length).toBe(1);
});
it('should warn if accessing any property on a fragment', function() {
src/isomorphic/classic/element/ReactElement.js
+19 -21
View File
@@ -80,21 +80,6 @@ function defineMutationMembrane(prototype) {
}
}
function markChildArrayValidated(childArray) {
// To make comparing ReactElements easier for testing purposes, we make the
// validation flag non-enumerable (where possible, which should include every
// environment we run tests in), so the test framework ignores it.
try {
Object.defineProperty(childArray, '_reactChildKeysValidated', {
configurable: false,
enumerable: false,
writable: true
});
} catch (x) {
}
childArray._reactChildKeysValidated = true;
}
/**
* Base constructor for all React elements. This is only used to make this
* 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) {
// commonly used development environments.
this._store = {props: props, originalProps: assign({}, props)};
// To make comparing ReactElements easier for testing purposes, we make
// the validation flag non-enumerable (where possible, which should
// include every environment we run tests in), so the test framework
// ignores it.
try {
Object.defineProperty(this._store, 'validated', {
configurable: false,
enumerable: false,
writable: true
});
} catch (x) {
}
this._store.validated = false;
// We're not allowed to set props directly on the object so we early
// return and rely on the prototype membrane to forward to the backing
// store.
@@ -171,9 +170,6 @@ ReactElement.createElement = function(type, config, children) {
props.children = children;
} else if (childrenLength > 1) {
var childArray = Array(childrenLength);
if (__DEV__) {
markChildArrayValidated(childArray);
}
for (var i = 0; i < childrenLength; i++) {
childArray[i] = arguments[i + 2];
}
@@ -220,6 +216,11 @@ ReactElement.cloneAndReplaceProps = function(oldElement, newProps) {
oldElement._context,
newProps
);
if (__DEV__) {
// If the key on the original is valid, then the clone is valid
newElement._store.validated = oldElement._store.validated;
}
return newElement;
};
@@ -261,9 +262,6 @@ ReactElement.cloneElement = function(element, config, children) {
props.children = children;
} else if (childrenLength > 1) {
var childArray = Array(childrenLength);
if (__DEV__) {
markChildArrayValidated(childArray);
}
for (var i = 0; i < childrenLength; i++) {
childArray[i] = arguments[i + 2];
}
src/isomorphic/classic/element/ReactElementValidator.js
+15 -37
View File
@@ -90,19 +90,15 @@ function getCurrentOwnerDisplayName() {
* @internal
* @param {ReactElement} element Element that requires a key.
* @param {*} parentType element's parent's type.
* @param {boolean} deep false if top-level collection, true if nested
*/
function validateExplicitKey(element, parentType, deep) {
if (element.key != null) {
function validateExplicitKey(element, parentType) {
if (element._store.validated || element.key != null) {
return;
}
element._store.validated = true;
warnAndMonitorForKeyUse(
// We vary the message for nested key warning to allow filtering them out
// since we didn't historically warn in this case.
deep ?
'Each child in a nested array or iterator should have ' +
'a unique "key" prop.' :
'Each child in an array or iterator should have a unique "key" prop.',
'Each child in an array or iterator should have a unique "key" prop.',
element,
parentType
);
@@ -116,17 +112,13 @@ function validateExplicitKey(element, parentType, deep) {
* @param {string} name Property name of the key.
* @param {ReactElement} element Component that requires a key.
* @param {*} parentType element's parent's type.
* @param {boolean} deep false if top-level collection, true if nested
*/
function validatePropertyKey(name, element, parentType, deep) {
function validatePropertyKey(name, element, parentType) {
if (!NUMERIC_PROPERTY_REGEX.test(name)) {
return;
}
warnAndMonitorForKeyUse(
deep ?
'Nested child objects should have non-numeric keys ' +
'so ordering is preserved.' :
'Child objects should have non-numeric keys so ordering is preserved.',
'Child objects should have non-numeric keys so ordering is preserved.',
element,
parentType
);
@@ -188,29 +180,18 @@ function warnAndMonitorForKeyUse(message, element, parentType) {
* @internal
* @param {ReactNode} node Statically passed child of any type.
* @param {*} parentType node's parent's type.
* @param {boolean} deep false if top-level collection, true if nested
*/
function validateChildKeys(node, parentType, deep) {
function validateChildKeys(node, parentType) {
if (Array.isArray(node)) {
if (node._reactChildKeysValidated) {
// All child elements were passed in a valid location.
return;
}
for (var i = 0; i < node.length; i++) {
var child = node[i];
if (ReactElement.isValidElement(child)) {
validateExplicitKey(child, parentType, deep);
} else {
// TODO: Warn on unkeyed arrays and suggest using createFragment
validateChildKeys(child, parentType, true);
validateExplicitKey(child, parentType);
}
}
} else if (
typeof node === 'string' || typeof node === 'number' ||
ReactElement.isValidElement(node)
) {
} else if (ReactElement.isValidElement(node)) {
// This element was passed in a valid location.
return;
node._store.validated = true;
} else if (node) {
var iteratorFn = getIteratorFn(node);
// Entry iterators provide implicit keys.
@@ -220,9 +201,7 @@ function validateChildKeys(node, parentType, deep) {
var step;
while (!(step = iterator.next()).done) {
if (ReactElement.isValidElement(step.value)) {
validateExplicitKey(step.value, parentType, deep);
} else {
validateChildKeys(step.value, parentType, true);
validateExplicitKey(step.value, parentType);
}
}
}
@@ -230,8 +209,7 @@ function validateChildKeys(node, parentType, deep) {
var fragment = ReactFragment.extractIfFragment(node);
for (var key in fragment) {
if (fragment.hasOwnProperty(key)) {
validatePropertyKey(key, fragment[key], parentType, deep);
validateChildKeys(fragment[key], parentType, true);
validatePropertyKey(key, fragment[key], parentType);
}
}
}
@@ -437,7 +415,7 @@ var ReactElementValidator = {
}
for (var i = 2; i < arguments.length; i++) {
validateChildKeys(arguments[i], type, false);
validateChildKeys(arguments[i], type);
}
validatePropTypes(element);
@@ -485,7 +463,7 @@ var ReactElementValidator = {
cloneElement: function(element, props, children) {
var newElement = ReactElement.cloneElement.apply(this, arguments);
for (var i = 2; i < arguments.length; i++) {
validateChildKeys(arguments[i], newElement.type, false);
validateChildKeys(arguments[i], newElement.type);
}
validatePropTypes(newElement);
return newElement;
src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js
+17 -50
View File
@@ -120,56 +120,6 @@ describe('ReactElementValidator', function() {
);
});
it('warns for keys for nested arrays of elements', function() {
spyOn(console, 'error');
var divs = [
[
<div />,
<div />
],
<div key="foo" />
];
ReactTestUtils.renderIntoDocument(<div>{divs}</div>);
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: Each child in a nested array or iterator should have a ' +
'unique "key" prop. Check the React.render call using <div>. See ' +
'https://fb.me/react-warning-keys for more information.'
);
});
it('warns for keys when reusing children', function() {
spyOn(console, 'error');
var f = <span />;
var g = <span />;
var children = [f, g];
ReactTestUtils.renderIntoDocument(
<div>
<div key="0">
{g}
</div>
<div key="1">
{f}
</div>
<div key="2">
{children}
</div>
</div>
);
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: Each child in an array or iterator should have a unique ' +
'"key" prop. Check the React.render call using <div>. See ' +
'https://fb.me/react-warning-keys for more information.'
);
});
it('does not warn for keys when passing children down', function() {
spyOn(console, 'error');
@@ -525,4 +475,21 @@ describe('ReactElementValidator', function() {
expect(console.error.argsForCall.length).toBe(1);
});
it('does not warn when using DOM node as children', function() {
spyOn(console, 'error');
var DOMContainer = React.createClass({
render: function() {
return <div />;
},
componentDidMount: function() {
React.findDOMNode(this).appendChild(this.props.children);
}
});
var node = document.createElement('div');
// This shouldn't cause a stack overflow or any other problems (#3883)
ReactTestUtils.renderIntoDocument(<DOMContainer>{node}</DOMContainer>);
expect(console.error.argsForCall.length).toBe(0);
});
});