diff --git a/src/addons/ReactFragment.js b/src/addons/ReactFragment.js index cb32b43566..d5862dd972 100644 --- a/src/addons/ReactFragment.js +++ b/src/addons/ReactFragment.js @@ -13,6 +13,8 @@ var ReactElement = require('ReactElement'); +var invariant = require('invariant'); +var traverseAllChildren = require('traverseAllChildren'); var warning = require('warning'); /** @@ -20,86 +22,26 @@ var warning = require('warning'); * or nested sets. This allowed us a way to explicitly key a set a fragment of * components. This is now being replaced with an opaque data structure. * The upgrade path is to call React.addons.createFragment({ key: value }) to - * create a keyed fragment. The resulting data structure is opaque, for now. + * create a keyed fragment. The resulting data structure is an array. */ -var fragmentKey; -var didWarnKey; -var canWarnForReactFragment; +var numericPropertyRegex = /^\d+$/; -if (__DEV__) { - fragmentKey = '_reactFragment'; - didWarnKey = '_reactDidWarn'; - - try { - // Feature test. Don't even try to issue this warning if we can't use - // enumerable: false. - - var dummy = function() { - return 1; - }; - - Object.defineProperty( - {}, - fragmentKey, - {enumerable: false, value: true} - ); - - Object.defineProperty( - {}, - 'key', - {enumerable: true, get: dummy} - ); - - canWarnForReactFragment = true; - } catch (x) { - canWarnForReactFragment = false; - } - - var proxyPropertyAccessWithWarning = function(obj, key) { - Object.defineProperty(obj, key, { - enumerable: true, - get: function() { - warning( - this[didWarnKey], - 'A ReactFragment is an opaque type. Accessing any of its ' + - 'properties is deprecated. Pass it to one of the React.Children ' + - 'helpers.' - ); - this[didWarnKey] = true; - return this[fragmentKey][key]; - }, - set: function(value) { - warning( - this[didWarnKey], - 'A ReactFragment is an immutable opaque type. Mutating its ' + - 'properties is deprecated.' - ); - this[didWarnKey] = true; - this[fragmentKey][key] = value; - }, - }); - }; - - var issuedWarnings = {}; - - var getFragmentKeyString = function(fragment) { - var fragmentCacheKey = ''; - for (var key in fragment) { - fragmentCacheKey += key + ':' + (typeof fragment[key]) + ','; - } - return fragmentCacheKey; - }; - - var didWarnForFragment = function(fragmentCacheKey) { - // We use the keys and the type of the value as a heuristic to dedupe the - // warning to avoid spamming too much. - var alreadyWarnedOnce = !!issuedWarnings[fragmentCacheKey]; - issuedWarnings[fragmentCacheKey] = true; - return alreadyWarnedOnce; - }; +var userProvidedKeyEscapeRegex = /\//g; +function escapeUserProvidedKey(text) { + return ('' + text).replace(userProvidedKeyEscapeRegex, '//'); } +function processSingleChildWithContext(ctx, child, childKey) { + if (ReactElement.isValidElement(child)) { + child = ReactElement.cloneAndReplaceKey(child, ctx.prefix + childKey); + } + // For text components, leave unkeyed + ctx.result.push(child); +} + +var warnedAboutNumeric = false; + var ReactFragment = { // Wrap a keyed object in an opaque proxy that warns you if you access any // of its properties. @@ -121,71 +63,35 @@ var ReactFragment = { ); return object; } - if (canWarnForReactFragment) { - var proxy = {}; - Object.defineProperty(proxy, fragmentKey, { - enumerable: false, - value: object, - }); - Object.defineProperty(proxy, didWarnKey, { - writable: true, - enumerable: false, - value: false, - }); - for (var key in object) { - proxyPropertyAccessWithWarning(proxy, key); - } - Object.preventExtensions(proxy); - return proxy; - } } - return object; - }, - // Extract the original keyed object from the fragment opaque type. Warn if - // a plain object is passed here. - extract: function(fragment) { - if (__DEV__) { - if (canWarnForReactFragment) { - if (!fragment[fragmentKey]) { - var fragmentKeys = getFragmentKeyString(fragment); + invariant( + object.nodeType !== 1, + 'React.addons.createFragment(...): Encountered an invalid child; DOM ' + + 'elements are not valid children of React components.' + ); + + var result = []; + var context = { + result: result, + prefix: '', + }; + + for (var key in object) { + if (__DEV__) { + if (!warnedAboutNumeric && numericPropertyRegex.test(key)) { warning( - didWarnForFragment(fragmentKeys), - 'Any use of a keyed object should be wrapped in ' + - 'React.addons.createFragment(object) before being passed as a ' + - 'child. {%s}', - fragmentKeys + false, + 'React.addons.createFragment(...): Child objects should have ' + + 'non-numeric keys so ordering is preserved.' ); - return fragment; - } - return fragment[fragmentKey]; - } - } - return fragment; - }, - // Check if this is a fragment and if so, extract the keyed object. If it - // is a fragment-like object, warn that it should be wrapped. Ignore if we - // can't determine what kind of object this is. - extractIfFragment: function(fragment) { - if (__DEV__) { - if (canWarnForReactFragment) { - // If it is the opaque type, return the keyed object. - if (fragment[fragmentKey]) { - return fragment[fragmentKey]; - } - // Otherwise, check each property if it has an element, if it does - // it is probably meant as a fragment, so we can warn early. Defer, - // the warning to extract. - for (var key in fragment) { - if (fragment.hasOwnProperty(key) && - ReactElement.isValidElement(fragment[key])) { - // This looks like a fragment object, we should provide an - // early warning. - return ReactFragment.extract(fragment); - } + warnedAboutNumeric = true; } } + context.prefix = escapeUserProvidedKey(key) + '/'; + traverseAllChildren(object[key], processSingleChildWithContext, context); } - return fragment; + + return result; }, }; diff --git a/src/addons/__tests__/ReactFragment-test.js b/src/addons/__tests__/ReactFragment-test.js index 25db57467e..5bb71bb264 100644 --- a/src/addons/__tests__/ReactFragment-test.js +++ b/src/addons/__tests__/ReactFragment-test.js @@ -23,51 +23,23 @@ describe('ReactFragment', function() { ReactFragment = require('ReactFragment'); }); - it('should warn if a plain object is used as a child', function() { - spyOn(console, 'error'); - var children = { - x: , - y: , - }; - void
{children}
; - expect(console.error.calls.length).toBe(1); - expect(console.error.calls[0].args[0]).toContain( - 'Any use of a keyed object' - ); - expect(console.error.calls[0].args[0]).toContain( - '{x:object,y:object,}' - ); - // Only warn once for the same set of children - var sameChildren = { - x: , - y: , - }; - void
{sameChildren}
; - expect(console.error.calls.length).toBe(1); - }); - - it('should warn if a plain object even if it is deep', function() { - spyOn(console, 'error'); + it('should throw if a plain object is used as a child', function() { var children = { x: , y: , z: , }; var element =
{[children]}
; - expect(console.error.calls.length).toBe(0); var container = document.createElement('div'); - ReactDOM.render(element, container); - expect(console.error.calls.length).toBe(1); - expect(console.error.calls[0].args[0]).toContain( - 'Any use of a keyed object' - ); - expect(console.error.calls[0].args[0]).toContain( - '{x:object,y:object,z:object,}' + expect(() => ReactDOM.render(element, container)).toThrow( + 'Invariant Violation: Objects are not valid as a React child (found ' + + 'object with keys {x, y, z}). If you meant to render a collection of ' + + 'children, use an array instead or wrap the object using ' + + 'React.addons.createFragment(object).' ); }); - it('should warn if a plain object even if it is in an owner', function() { - spyOn(console, 'error'); + it('should throw if a plain object even if it is in an owner', function() { class Foo { render() { var children = { @@ -78,27 +50,23 @@ describe('ReactFragment', function() { return
{[children]}
; } } - expect(console.error.calls.length).toBe(0); var container = document.createElement('div'); - ReactDOM.render(, container); - expect(console.error.calls.length).toBe(1); - expect(console.error.calls[0].args[0]).toContain( - 'Any use of a keyed object' + expect(() => ReactDOM.render(, container)).toThrow( + 'Invariant Violation: Objects are not valid as a React child (found ' + + 'object with keys {a, b, c}). If you meant to render a collection of ' + + 'children, use an array instead or wrap the object using ' + + 'React.addons.createFragment(object). Check the render method of `Foo`.' ); }); - it('should warn if accessing any property on a fragment', function() { + it('warns for numeric keys on objects as children', function() { spyOn(console, 'error'); - var children = { - x: , - y: , - }; - var frag = ReactFragment.create(children); - void frag.x; - frag.y = 10; - expect(console.error.calls.length).toBe(1); - expect(console.error.calls[0].args[0]).toContain( - 'A ReactFragment is an opaque type' + + ReactFragment.create({1: , 2: }); + + expect(console.error.argsForCall.length).toBe(1); + expect(console.error.argsForCall[0][0]).toContain( + 'Child objects should have non-numeric keys so ordering is preserved.' ); }); diff --git a/src/addons/transitions/ReactTransitionChildMapping.js b/src/addons/transitions/ReactTransitionChildMapping.js index 7bea6b55a2..f0b9e90177 100644 --- a/src/addons/transitions/ReactTransitionChildMapping.js +++ b/src/addons/transitions/ReactTransitionChildMapping.js @@ -12,13 +12,12 @@ 'use strict'; -var ReactChildren = require('ReactChildren'); -var ReactFragment = require('ReactFragment'); +var flattenChildren = require('flattenChildren'); var ReactTransitionChildMapping = { /** * Given `this.props.children`, return an object mapping key to child. Just - * simple syntactic sugar around ReactChildren.map(). + * simple syntactic sugar around flattenChildren(). * * @param {*} children `this.props.children` * @return {object} Mapping of key to child @@ -27,9 +26,7 @@ var ReactTransitionChildMapping = { if (!children) { return children; } - return ReactFragment.extract(ReactChildren.map(children, function(child) { - return child; - })); + return flattenChildren(children); }, /** diff --git a/src/isomorphic/children/ReactChildren.js b/src/isomorphic/children/ReactChildren.js index c17d502a25..9abb0d966a 100644 --- a/src/isomorphic/children/ReactChildren.js +++ b/src/isomorphic/children/ReactChildren.js @@ -146,7 +146,7 @@ function countChildren(children, context) { } -function pushSingleChildToArray(traverseContext, child, name, i) { +function pushSingleChildToArray(traverseContext, child, name) { if (child == null) { return; } diff --git a/src/isomorphic/children/__tests__/ReactChildren-test.js b/src/isomorphic/children/__tests__/ReactChildren-test.js index 3f6c1cf058..2c4abe8080 100644 --- a/src/isomorphic/children/__tests__/ReactChildren-test.js +++ b/src/isomorphic/children/__tests__/ReactChildren-test.js @@ -22,20 +22,6 @@ describe('ReactChildren', function() { ReactFragment = require('ReactFragment'); }); - function nthChild(mappedChildren, n) { - var result = null; - ReactChildren.forEach(mappedChildren, function(child, index) { - if (index === n) { - result = child; - } - }); - return result; - } - - function keyOfNthChild(mappedChildren, n) { - return Object.keys(ReactFragment.extract(mappedChildren))[n]; - } - it('should support identity for simple', function() { var callback = jasmine.createSpy().andCallFake(function(kid, index) { return kid; @@ -52,7 +38,7 @@ describe('ReactChildren', function() { callback.reset(); var mappedChildren = ReactChildren.map(instance.props.children, callback); expect(callback).toHaveBeenCalledWith(simpleKid, 0); - expect(nthChild(mappedChildren, 0)).toBe(simpleKid); + expect(mappedChildren[0]).toEqual(); }); it('should treat single arrayless child as being in array', function() { @@ -67,7 +53,7 @@ describe('ReactChildren', function() { callback.reset(); var mappedChildren = ReactChildren.map(instance.props.children, callback); expect(callback).toHaveBeenCalledWith(simpleKid, 0); - expect(nthChild(mappedChildren, 0)).toBe(simpleKid); + expect(mappedChildren[0]).toEqual(); }); it('should treat single child in array as expected', function() { @@ -82,7 +68,7 @@ describe('ReactChildren', function() { callback.reset(); var mappedChildren = ReactChildren.map(instance.props.children, callback); expect(callback).toHaveBeenCalledWith(simpleKid, 0); - expect(nthChild(mappedChildren, 0)).toBe(simpleKid); + expect(mappedChildren[0]).toEqual(); }); @@ -97,9 +83,9 @@ describe('ReactChildren', function() { var mappedChildren = ReactChildren.map(instance.props.children, mapFn); expect(ReactChildren.count(mappedChildren)).toBe(1); - expect(nthChild(mappedChildren, 0)).not.toBe(simpleKid); - expect(nthChild(mappedChildren, 0).props.children).toBe(simpleKid); - expect(keyOfNthChild(mappedChildren, 0)).toBe('.$simple'); + expect(mappedChildren[0]).not.toBe(simpleKid); + expect(mappedChildren[0].props.children).toBe(simpleKid); + expect(mappedChildren[0].key).toBe('.$simple/.0'); }); it('should invoke callback with the right context', function() { @@ -121,7 +107,7 @@ describe('ReactChildren', function() { ReactChildren.map(instance.props.children, callback, scopeTester); expect(ReactChildren.count(mappedChildren)).toBe(1); - expect(nthChild(mappedChildren, 0)).toBe(scopeTester); + expect(mappedChildren[0]).toBe(scopeTester); }); it('should be called for each child', function() { @@ -165,35 +151,30 @@ describe('ReactChildren', function() { var mappedChildren = ReactChildren.map(instance.props.children, callback); expect(callback.calls.length).toBe(5); - expect(ReactChildren.count(mappedChildren)).toBe(5); + expect(ReactChildren.count(mappedChildren)).toBe(4); // Keys default to indices. expect([ - keyOfNthChild(mappedChildren, 0), - keyOfNthChild(mappedChildren, 1), - keyOfNthChild(mappedChildren, 2), - keyOfNthChild(mappedChildren, 3), - keyOfNthChild(mappedChildren, 4), + mappedChildren[0].key, + mappedChildren[1].key, + mappedChildren[2].key, + mappedChildren[3].key, ]).toEqual( - ['.$keyZero', '.1', '.$keyTwo', '.3', '.$keyFour'] + ['.$keyZero/.$giraffe', '.$keyTwo/.0', '.3/.0', '.$keyFour/.$keyFour'] ); expect(callback).toHaveBeenCalledWith(zero, 0); - expect(nthChild(mappedChildren, 0)).toBe(zeroMapped); + expect(mappedChildren[0]).toEqual(
); expect(callback).toHaveBeenCalledWith(one, 1); - expect(nthChild(mappedChildren, 1)).toBe(oneMapped); + expect(mappedChildren[1]).toEqual(
); expect(callback).toHaveBeenCalledWith(two, 2); - expect(nthChild(mappedChildren, 2)).toBe(twoMapped); + expect(mappedChildren[2]).toEqual(); expect(callback).toHaveBeenCalledWith(three, 3); - expect(nthChild(mappedChildren, 3)).toBe(threeMapped); - - expect(callback).toHaveBeenCalledWith(four, 4); - expect(nthChild(mappedChildren, 4)).toBe(fourMapped); + expect(mappedChildren[3]).toEqual(
); }); - it('should be called for each child in nested structure', function() { var zero =
; var one = null; @@ -222,64 +203,54 @@ describe('ReactChildren', function() { index === 4 ? fourMapped : fiveMapped; }); - var instance = ( -
{ - [ - ReactFragment.create({ - firstHalfKey: [zero, one, two], - secondHalfKey: [three, four], - keyFive: five, - }), - ] - }
- ); + var frag = ReactFragment.create({ + firstHalfKey: [zero, one, two], + secondHalfKey: [three, four], + keyFive: five, + }); + var instance =
{[frag]}
; ReactChildren.forEach(instance.props.children, callback); - expect(callback).toHaveBeenCalledWith(zero, 0); - expect(callback).toHaveBeenCalledWith(one, 1); - expect(callback).toHaveBeenCalledWith(two, 2); - expect(callback).toHaveBeenCalledWith(three, 3); - expect(callback).toHaveBeenCalledWith(four, 4); - expect(callback).toHaveBeenCalledWith(five, 5); + expect(callback).toHaveBeenCalledWith(frag[0], 0); + expect(callback).toHaveBeenCalledWith(frag[1], 1); + expect(callback).toHaveBeenCalledWith(frag[2], 2); + expect(callback).toHaveBeenCalledWith(frag[3], 3); + expect(callback).toHaveBeenCalledWith(frag[4], 4); + expect(callback).toHaveBeenCalledWith(frag[5], 5); callback.reset(); var mappedChildren = ReactChildren.map(instance.props.children, callback); expect(callback.calls.length).toBe(6); - expect(ReactChildren.count(mappedChildren)).toBe(6); + expect(ReactChildren.count(mappedChildren)).toBe(5); // Keys default to indices. expect([ - keyOfNthChild(mappedChildren, 0), - keyOfNthChild(mappedChildren, 1), - keyOfNthChild(mappedChildren, 2), - keyOfNthChild(mappedChildren, 3), - keyOfNthChild(mappedChildren, 4), - keyOfNthChild(mappedChildren, 5), + mappedChildren[0].key, + mappedChildren[1].key, + mappedChildren[2].key, + mappedChildren[3].key, + mappedChildren[4].key, ]).toEqual([ - '.0:$firstHalfKey:0:$keyZero', - '.0:$firstHalfKey:0:1', - '.0:$firstHalfKey:0:$keyTwo', - '.0:$secondHalfKey:0:0', - '.0:$secondHalfKey:0:$keyFour', - '.0:$keyFive:$keyFiveInner', + '.0:$firstHalfKey//=1$keyZero/.$giraffe', + '.0:$firstHalfKey//=1$keyTwo/.0', + '.0:3/.0', + '.0:$secondHalfKey//=1$keyFour/.$keyFour', + '.0:$keyFive//=1$keyFiveInner/.0', ]); - expect(callback).toHaveBeenCalledWith(zero, 0); - expect(nthChild(mappedChildren, 0)).toBe(zeroMapped); + expect(callback).toHaveBeenCalledWith(frag[0], 0); + expect(mappedChildren[0]).toEqual(
); - expect(callback).toHaveBeenCalledWith(one, 1); - expect(nthChild(mappedChildren, 1)).toBe(oneMapped); + expect(callback).toHaveBeenCalledWith(frag[1], 1); + expect(mappedChildren[1]).toEqual(
); - expect(callback).toHaveBeenCalledWith(two, 2); - expect(nthChild(mappedChildren, 2)).toBe(twoMapped); + expect(callback).toHaveBeenCalledWith(frag[2], 2); + expect(mappedChildren[2]).toEqual(); - expect(callback).toHaveBeenCalledWith(three, 3); - expect(nthChild(mappedChildren, 3)).toBe(threeMapped); + expect(callback).toHaveBeenCalledWith(frag[3], 3); + expect(mappedChildren[3]).toEqual(
); - expect(callback).toHaveBeenCalledWith(four, 4); - expect(nthChild(mappedChildren, 4)).toBe(fourMapped); - - expect(callback).toHaveBeenCalledWith(five, 5); - expect(nthChild(mappedChildren, 5)).toBe(fiveMapped); + expect(callback).toHaveBeenCalledWith(frag[4], 4); + expect(mappedChildren[4]).toEqual(
); }); it('should retain key across two mappings', function() { @@ -302,20 +273,20 @@ describe('ReactChildren', function() {
); - var expectedForcedKeys = ['.$keyZero', '.$keyOne']; + var expectedForcedKeys = ['.$keyZero/.$giraffe', '.$keyOne/.0']; var mappedChildrenForcedKeys = ReactChildren.map(forcedKeys.props.children, mapFn); - var mappedForcedKeys = Object.keys(mappedChildrenForcedKeys); + var mappedForcedKeys = mappedChildrenForcedKeys.map((c) => c.key); expect(mappedForcedKeys).toEqual(expectedForcedKeys); var expectedRemappedForcedKeys = [ - '.$=1$keyZero:$giraffe', - '.$=1$keyOne:0', + '.$=1$keyZero//=1$giraffe/.$giraffe', + '.$=1$keyOne//=10/.0', ]; var remappedChildrenForcedKeys = ReactChildren.map(mappedChildrenForcedKeys, mapFn); expect( - Object.keys(remappedChildrenForcedKeys) + remappedChildrenForcedKeys.map((c) => c.key) ).toEqual(expectedRemappedForcedKeys); }); @@ -355,8 +326,7 @@ describe('ReactChildren', function() { var mapped = ReactChildren.map(instance.props.children, mapFn); expect(console.error.calls.length).toEqual(1); - expect(nthChild(mapped, 0)).toBe(zero); - expect(keyOfNthChild(mapped, 0)).toBe('.$something'); + expect(mapped[0]).toEqual(
); }); it('should return 0 for null children', function() { @@ -424,40 +394,6 @@ describe('ReactChildren', function() { expect(numberOfChildren).toBe(6); }); - it('should warn if a fragment is used without the wrapper', function() { - spyOn(console, 'error'); - var child = React.createElement('span'); - ReactChildren.forEach({a: child, b: child}, function(c) { - expect(c).toBe(child); - }); - expect(console.error.calls.length).toBe(1); - expect(console.error.calls[0].args[0]).toContain('use of a keyed object'); - }); - - it('should warn if a fragment is accessed', function() { - spyOn(console, 'error'); - var child = React.createElement('span'); - var frag = ReactChildren.map([child, child], function(c) { - return c; - }); - for (var key in frag) { - void frag[key]; - break; - } - expect(console.error.calls.length).toBe(1); - expect(console.error.calls[0].args[0]).toContain('is an opaque type'); - - var frag2 = ReactChildren.map([child, child], function(c) { - return c; - }); - for (var key2 in frag2) { - frag2[key2] = 123; - break; - } - expect(console.error.calls.length).toBe(2); - expect(console.error.calls[1].args[0]).toContain('is an immutable opaque'); - }); - it('should flatten children to an array', function() { expect(ReactChildren.toArray(undefined)).toEqual([]); expect(ReactChildren.toArray(null)).toEqual([]); diff --git a/src/isomorphic/children/__tests__/sliceChildren-test.js b/src/isomorphic/children/__tests__/sliceChildren-test.js index 385e4ea142..0f9cbcff13 100644 --- a/src/isomorphic/children/__tests__/sliceChildren-test.js +++ b/src/isomorphic/children/__tests__/sliceChildren-test.js @@ -14,22 +14,15 @@ describe('sliceChildren', function() { var React; - var ReactFragment; var sliceChildren; beforeEach(function() { React = require('React'); - ReactFragment = require('ReactFragment'); sliceChildren = require('sliceChildren'); }); - function testKeyValuePairs(children, expectedPairs) { - var obj = ReactFragment.extract(children); - expect(obj).toEqual(expectedPairs); - } - it('should render the whole set if start zero is supplied', function() { var fullSet = [
, @@ -37,11 +30,11 @@ describe('sliceChildren', function() {
, ]; var children = sliceChildren(fullSet, 0); - testKeyValuePairs(children, { - '.$A': fullSet[0], - '.$B': fullSet[1], - '.$C': fullSet[2], - }); + expect(children).toEqual([ +
, +
, +
, + ]); }); it('should render the remaining set if no end index is supplied', function() { @@ -51,10 +44,10 @@ describe('sliceChildren', function() {
, ]; var children = sliceChildren(fullSet, 1); - testKeyValuePairs(children, { - '.$B': fullSet[1], - '.$C': fullSet[2], - }); + expect(children).toEqual([ +
, +
, + ]); }); it('should exclude everything at or after the end index', function() { @@ -65,21 +58,36 @@ describe('sliceChildren', function() {
, ]; var children = sliceChildren(fullSet, 1, 2); - testKeyValuePairs(children, { - '.$B': fullSet[1], - }); + expect(children).toEqual([ +
, + ]); }); it('should allow static children to be sliced', function() { - var a =
; - var b =
; - var c =
; + var a = ; + var b = ; + var c = ; var el =
{a}{b}{c}
; var children = sliceChildren(el.props.children, 1, 2); - testKeyValuePairs(children, { - '.1': b, - }); + expect(children).toEqual([ + , + ]); + }); + + it('should slice nested children', function() { + var fullSet = [ +
, + [ +
, +
, + ], +
, + ]; + var children = sliceChildren(fullSet, 1, 2); + expect(children).toEqual([ +
, + ]); }); }); diff --git a/src/isomorphic/children/sliceChildren.js b/src/isomorphic/children/sliceChildren.js index 22a01dea14..35a2af0bf0 100644 --- a/src/isomorphic/children/sliceChildren.js +++ b/src/isomorphic/children/sliceChildren.js @@ -11,9 +11,7 @@ 'use strict'; -var ReactFragment = require('ReactFragment'); - -var flattenChildren = require('flattenChildren'); +var ReactChildren = require('ReactChildren'); /** * Slice children that are typically specified as `props.children`. This version @@ -29,23 +27,8 @@ function sliceChildren(children, start, end) { return children; } - var slicedChildren = {}; - var flattenedMap = flattenChildren(children); - var ii = 0; - for (var key in flattenedMap) { - if (!flattenedMap.hasOwnProperty(key)) { - continue; - } - var child = flattenedMap[key]; - if (ii >= start) { - slicedChildren[key] = child; - } - ii++; - if (end != null && ii >= end) { - break; - } - } - return ReactFragment.create(slicedChildren); + var array = ReactChildren.toArray(children); + return array.slice(start, end); } module.exports = sliceChildren; diff --git a/src/isomorphic/classic/element/ReactElementValidator.js b/src/isomorphic/classic/element/ReactElementValidator.js index 203bab1ee2..b3af56bb25 100644 --- a/src/isomorphic/classic/element/ReactElementValidator.js +++ b/src/isomorphic/classic/element/ReactElementValidator.js @@ -19,7 +19,6 @@ 'use strict'; var ReactElement = require('ReactElement'); -var ReactFragment = require('ReactFragment'); var ReactPropTypeLocations = require('ReactPropTypeLocations'); var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames'); var ReactCurrentOwner = require('ReactCurrentOwner'); @@ -47,8 +46,6 @@ var ownerHasKeyUseWarning = {}; var loggedTypeFailures = {}; -var NUMERIC_PROPERTY_REGEX = /^\d+$/; - /** * Warn if the element doesn't have an explicit key assigned to it. * This element is in an array. The array could grow and shrink or be @@ -80,34 +77,6 @@ function validateExplicitKey(element, parentType) { ); } -/** - * Warn if the key is being defined as an object property but has an incorrect - * value. - * - * @internal - * @param {string} name Property name of the key. - * @param {ReactElement} element Component that requires a key. - * @param {*} parentType element's parent's type. - */ -function validatePropertyKey(name, element, parentType) { - if (!NUMERIC_PROPERTY_REGEX.test(name)) { - return; - } - var addenda = getAddendaForKeyUse('numericKeys', element, parentType); - if (addenda === null) { - // we already showed the warning - return; - } - warning( - false, - 'Child objects should have non-numeric keys so ordering is preserved.' + - '%s%s%s', - addenda.parentOrOwner || '', - addenda.childOwner || '', - addenda.url || '' - ); -} - /** * Shared warning and monitoring code for the key warnings. * @@ -189,13 +158,6 @@ function validateChildKeys(node, parentType) { } } } - } else if (typeof node === 'object') { - var fragment = ReactFragment.extractIfFragment(node); - for (var key in fragment) { - if (fragment.hasOwnProperty(key)) { - validatePropertyKey(key, fragment[key], parentType); - } - } } } } diff --git a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js index 5710819882..b09b661b5d 100644 --- a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js +++ b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js @@ -16,7 +16,6 @@ var React; var ReactDOM; -var ReactFragment; var ReactTestUtils; describe('ReactElementValidator', function() { @@ -27,7 +26,6 @@ describe('ReactElementValidator', function() { React = require('React'); ReactDOM = require('ReactDOM'); - ReactFragment = require('ReactFragment'); ReactTestUtils = require('ReactTestUtils'); ComponentClass = React.createClass({ render: function() { @@ -36,10 +34,6 @@ describe('ReactElementValidator', function() { }); }); - function frag(obj) { - return ReactFragment.create(obj); - } - it('warns for keys for arrays of elements in rest args', function() { spyOn(console, 'error'); var Component = React.createFactory(ComponentClass); @@ -205,41 +199,6 @@ describe('ReactElementValidator', function() { expect(console.error.argsForCall.length).toBe(0); }); - it('warns for numeric keys on objects in rest args', function() { - spyOn(console, 'error'); - var Component = React.createFactory(ComponentClass); - - Component(null, frag({1: Component(), 2: Component()})); - - expect(console.error.argsForCall.length).toBe(1); - expect(console.error.argsForCall[0][0]).toContain( - 'Child objects should have non-numeric keys so ordering is preserved.' - ); - }); - - it('does not warn for numeric keys in entry iterables in rest args', - function() { - spyOn(console, 'error'); - var Component = React.createFactory(ComponentClass); - - var iterable = { - '@@iterator': function() { - var i = 0; - return { - next: function() { - var done = ++i > 2; - return {value: done ? undefined : [i, Component()], done: done}; - }, - }; - }, - }; - iterable.entries = iterable['@@iterator']; - - Component(null, iterable); - - expect(console.error.argsForCall.length).toBe(0); - }); - it('does not warn when the element is directly in rest args', function() { spyOn(console, 'error'); var Component = React.createFactory(ComponentClass); @@ -454,14 +413,6 @@ describe('ReactElementValidator', function() { ); }); - it('should warn if a fragment is used without the wrapper', function() { - spyOn(console, 'error'); - var child = React.createElement('span'); - React.createElement('div', null, {a: child, b: child}); - expect(console.error.calls.length).toBe(1); - expect(console.error.calls[0].args[0]).toContain('use of a keyed object'); - }); - it('should warn when accessing .type on an element factory', function() { spyOn(console, 'error'); var TestComponent = React.createClass({ diff --git a/src/isomorphic/classic/types/ReactPropTypes.js b/src/isomorphic/classic/types/ReactPropTypes.js index 9400cd76fa..5e8b1d2391 100644 --- a/src/isomorphic/classic/types/ReactPropTypes.js +++ b/src/isomorphic/classic/types/ReactPropTypes.js @@ -12,7 +12,6 @@ 'use strict'; var ReactElement = require('ReactElement'); -var ReactFragment = require('ReactFragment'); var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames'); var emptyFunction = require('emptyFunction'); @@ -371,12 +370,7 @@ function isNode(propValue) { } } } else { - propValue = ReactFragment.extractIfFragment(propValue); - for (var k in propValue) { - if (!isNode(propValue[k])) { - return false; - } - } + return false; } return true; diff --git a/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js b/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js index 95ca5cac45..aa23672d92 100644 --- a/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js +++ b/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js @@ -368,6 +368,7 @@ describe('ReactPropTypes', function() { typeCheckFail(PropTypes.node, true, failMessage); typeCheckFail(PropTypes.node, function() {}, failMessage); typeCheckFail(PropTypes.node, {key: function() {}}, failMessage); + typeCheckFail(PropTypes.node, {key:
}, failMessage); }); it('should not warn for valid values', function() { @@ -377,7 +378,6 @@ describe('ReactPropTypes', function() { typeCheckPass(PropTypes.node, ); typeCheckPass(PropTypes.node, 'Some string'); typeCheckPass(PropTypes.node, []); - typeCheckPass(PropTypes.node, {}); typeCheckPass(PropTypes.node, [ 123, @@ -402,20 +402,6 @@ describe('ReactPropTypes', function() { k5: undefined, })); expect(console.error.calls).toEqual([]); - - // This should also pass, though it warns - typeCheckPass(PropTypes.node, { - k0: 123, - k1: 'Some string', - k2:
, - k3: { - k30: , - k31: {k310: }, - k32: 'Another string', - }, - k4: null, - k5: undefined, - }); }); it('should not warn for iterables', function() { diff --git a/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js b/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js index 2ad1ec5f77..97b86d4902 100644 --- a/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js +++ b/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js @@ -15,7 +15,6 @@ // of dynamic errors when using JSX with Flow. var React; -var ReactFragment; var ReactTestUtils; describe('ReactJSXElementValidator', function() { @@ -26,7 +25,6 @@ describe('ReactJSXElementValidator', function() { require('mock-modules').dumpCache(); React = require('React'); - ReactFragment = require('ReactFragment'); ReactTestUtils = require('ReactTestUtils'); Component = class { @@ -44,10 +42,6 @@ describe('ReactJSXElementValidator', function() { RequiredPropComponent.propTypes = {prop: React.PropTypes.string.isRequired}; }); - function frag(obj) { - return ReactFragment.create(obj); - } - it('warns for keys for arrays of elements in children position', function() { spyOn(console, 'error'); @@ -142,17 +136,6 @@ describe('ReactJSXElementValidator', function() { expect(console.error.argsForCall.length).toBe(0); }); - it('warns for numeric keys on objects as children', function() { - spyOn(console, 'error'); - - void {frag({1: , 2: })}; - - expect(console.error.argsForCall.length).toBe(1); - expect(console.error.argsForCall[0][0]).toContain( - 'Child objects should have non-numeric keys so ordering is preserved.' - ); - }); - it('does not warn for numeric keys in entry iterable as a child', function() { spyOn(console, 'error'); diff --git a/src/renderers/shared/reconciler/__tests__/ReactIdentity-test.js b/src/renderers/shared/reconciler/__tests__/ReactIdentity-test.js index 8609371307..4cabcd9c01 100644 --- a/src/renderers/shared/reconciler/__tests__/ReactIdentity-test.js +++ b/src/renderers/shared/reconciler/__tests__/ReactIdentity-test.js @@ -53,8 +53,8 @@ describe('ReactIdentity', function() { instance = ReactDOM.render(instance, document.createElement('div')); var node = ReactDOM.findDOMNode(instance); expect(node.childNodes.length).toBe(2); - checkID(node.childNodes[0], '.0.$first:0'); - checkID(node.childNodes[1], '.0.$second:0'); + checkID(node.childNodes[0], '.0.$first/=10'); + checkID(node.childNodes[1], '.0.$second/=10'); }); it('should allow key property to express identity', function() { @@ -125,9 +125,9 @@ describe('ReactIdentity', function() { expect(ReactDOM.findDOMNode(span2)).not.toBe(null); key = key.replace(/=/g, '=0'); - checkID(ReactDOM.findDOMNode(span1), '.0.$' + key); - checkID(ReactDOM.findDOMNode(span2), '.0.1:$' + key + ':0'); + key = key.replace(/\//g, '//'); + checkID(ReactDOM.findDOMNode(span2), '.0.1:$' + key + '/=10'); } it('should allow any character as a key, in a detached parent', function() { diff --git a/src/shared/utils/__tests__/traverseAllChildren-test.js b/src/shared/utils/__tests__/traverseAllChildren-test.js index 7c2f678f1a..26fb2e4580 100644 --- a/src/shared/utils/__tests__/traverseAllChildren-test.js +++ b/src/shared/utils/__tests__/traverseAllChildren-test.js @@ -162,10 +162,10 @@ describe('traverseAllChildren', function() { traverseContext, div, '.$divNode' ); expect(traverseFn).toHaveBeenCalledWith( - traverseContext, span, '.1:0:$span:$spanNode' + traverseContext, , '.1:0:$span/=1$spanNode' ); expect(traverseFn).toHaveBeenCalledWith( - traverseContext, a, '.2:$a:$aNode' + traverseContext, , '.2:$a/=1$aNode' ); expect(traverseFn).toHaveBeenCalledWith( traverseContext, 'string', '.3' @@ -224,35 +224,26 @@ describe('traverseAllChildren', function() { expect(traverseContext.length).toEqual(6); expect(traverseFn).toHaveBeenCalledWith( traverseContext, - zero, - '.0:$firstHalfKey:0:$keyZero' - ); - - expect(traverseFn) - .toHaveBeenCalledWith(traverseContext, one, '.0:$firstHalfKey:0:1'); - - expect(traverseFn).toHaveBeenCalledWith( - traverseContext, - two, - '.0:$firstHalfKey:0:$keyTwo' +
, + '.0:$firstHalfKey/=1$keyZero' ); expect(traverseFn).toHaveBeenCalledWith( traverseContext, - three, - '.0:$secondHalfKey:0:0' +
, + '.0:$firstHalfKey/=1$keyTwo' ); expect(traverseFn).toHaveBeenCalledWith( traverseContext, - four, - '.0:$secondHalfKey:0:$keyFour' +
, + '.0:$secondHalfKey/=1$keyFour' ); expect(traverseFn).toHaveBeenCalledWith( traverseContext, - five, - '.0:$keyFive:$keyFiveInner' +
, + '.0:$keyFive/=1$keyFiveInner' ); }); diff --git a/src/shared/utils/traverseAllChildren.js b/src/shared/utils/traverseAllChildren.js index b708ecb1aa..c9f19de131 100644 --- a/src/shared/utils/traverseAllChildren.js +++ b/src/shared/utils/traverseAllChildren.js @@ -11,8 +11,8 @@ 'use strict'; +var ReactCurrentOwner = require('ReactCurrentOwner'); var ReactElement = require('ReactElement'); -var ReactFragment = require('ReactFragment'); var ReactInstanceHandles = require('ReactInstanceHandles'); var getIteratorFn = require('getIteratorFn'); @@ -179,28 +179,24 @@ function traverseAllChildrenImpl( } } } else if (type === 'object') { - invariant( - children.nodeType !== 1, - 'traverseAllChildren(...): Encountered an invalid child; DOM ' + - 'elements are not valid children of React components.' - ); - var fragment = ReactFragment.extract(children); - for (var key in fragment) { - if (fragment.hasOwnProperty(key)) { - child = fragment[key]; - nextName = ( - nextNamePrefix + - wrapUserProvidedKey(key) + SUBSEPARATOR + - getComponentKey(child, 0) - ); - subtreeCount += traverseAllChildrenImpl( - child, - nextName, - callback, - traverseContext - ); + var addendum = ''; + if (__DEV__) { + if (ReactCurrentOwner.current) { + var name = ReactCurrentOwner.current.getName(); + if (name) { + addendum = ' Check the render method of `' + name + '`.' + } } } + invariant( + false, + 'Objects are not valid as a React child (found object with keys ' + + '{%s}). If you meant to render a collection of children, use an ' + + 'array instead or wrap the object using ' + + 'React.addons.createFragment(object).%s', + Object.keys(children).join(', '), + addendum + ); } }