diff --git a/src/addons/__tests__/ReactFragment-test.js b/src/addons/__tests__/ReactFragment-test.js index 6b789e016a..d53e6020de 100644 --- a/src/addons/__tests__/ReactFragment-test.js +++ b/src/addons/__tests__/ReactFragment-test.js @@ -49,13 +49,13 @@ describe('ReactFragment', function() { z: }; var element =
{[children]}
; + 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() { diff --git a/src/isomorphic/classic/element/ReactElement.js b/src/isomorphic/classic/element/ReactElement.js index 88d7f36c3e..4760c6311d 100644 --- a/src/isomorphic/classic/element/ReactElement.js +++ b/src/isomorphic/classic/element/ReactElement.js @@ -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]; } diff --git a/src/isomorphic/classic/element/ReactElementValidator.js b/src/isomorphic/classic/element/ReactElementValidator.js index 93d14798da..48b0f44d6d 100644 --- a/src/isomorphic/classic/element/ReactElementValidator.js +++ b/src/isomorphic/classic/element/ReactElementValidator.js @@ -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; diff --git a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js index fab2a7d789..2f8d265956 100644 --- a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js +++ b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js @@ -120,56 +120,6 @@ describe('ReactElementValidator', function() { ); }); - it('warns for keys for nested arrays of elements', function() { - spyOn(console, 'error'); - - var divs = [ - [ -
, -
- ], -
- ]; - ReactTestUtils.renderIntoDocument(
{divs}
); - - 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
. See ' + - 'https://fb.me/react-warning-keys for more information.' - ); - }); - - it('warns for keys when reusing children', function() { - spyOn(console, 'error'); - - var f = ; - var g = ; - - var children = [f, g]; - - ReactTestUtils.renderIntoDocument( -
-
- {g} -
-
- {f} -
-
- {children} -
-
- ); - - 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
. 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
; + }, + 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({node}); + expect(console.error.argsForCall.length).toBe(0); + }); + });