From 56f51156bab53e39b80397211d4b1ba853902ac4 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 9 Feb 2015 14:24:46 -0800 Subject: [PATCH] Make keyed object fragments an opaque type This triggers a warning if you try to pass a keyed object as a child. You now have to wrap it in React.addons.createFragment(object) which creates a proxy (in dev) which warns if it is accessed. The purpose of this is to make these into opaque objects so that nobody relies on its data structure. After that we can turn it into a different data structure such as a ReactFragment node or an iterable of flattened ReactElements. --- src/addons/ReactFragment.js | 164 ++++++++++++++++++ src/addons/__tests__/ReactFragment-test.js | 76 ++++++++ .../ReactTransitionChildMapping.js | 8 +- .../transitions/ReactTransitionGroup.js | 8 +- src/browser/ReactWithAddons.js | 2 + src/browser/__tests__/ReactDOM-test.js | 38 ++-- src/classic/element/ReactElementValidator.js | 38 ++-- .../__tests__/ReactElementValidator-test.js | 16 +- src/core/__tests__/ReactIdentity-test.js | 12 +- .../ReactMultiChildReconcile-test.js | 11 +- .../__tests__/ReactMultiChildText-test.js | 23 +-- .../ReactJSXElementValidator-test.js | 8 +- src/utils/ReactChildren.js | 3 +- src/utils/__tests__/ReactChildren-test.js | 133 ++++++++++---- src/utils/__tests__/onlyChild-test.js | 4 +- src/utils/__tests__/sliceChildren-test.js | 15 +- .../__tests__/traverseAllChildren-test.js | 13 +- src/utils/sliceChildren.js | 4 +- src/utils/traverseAllChildren.js | 8 +- 19 files changed, 466 insertions(+), 118 deletions(-) create mode 100644 src/addons/ReactFragment.js create mode 100644 src/addons/__tests__/ReactFragment-test.js diff --git a/src/addons/ReactFragment.js b/src/addons/ReactFragment.js new file mode 100644 index 0000000000..e6afbd75f4 --- /dev/null +++ b/src/addons/ReactFragment.js @@ -0,0 +1,164 @@ +/** + * Copyright 2015, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * +* @providesModule ReactFragment +*/ + +'use strict'; + +var ReactElement = require('ReactElement'); + +var warning = require('warning'); + +/** + * We used to allow keyed objects to serve as a collection of ReactElements, + * 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. + */ + +if (__DEV__) { + var fragmentKey = '_reactFragment'; + var didWarnKey = '_reactDidWarn'; + var canWarnForReactFragment = false; + + 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) { } + + 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 didWarnForFragment = function(fragment) { + // We use the keys and the type of the value as a heuristic to dedupe the + // warning to avoid spamming too much. + var fragmentCacheKey = ''; + for (var key in fragment) { + fragmentCacheKey += key + ':' + (typeof fragment[key]) + ','; + } + var alreadyWarnedOnce = !!issuedWarnings[fragmentCacheKey]; + issuedWarnings[fragmentCacheKey] = true; + return alreadyWarnedOnce; + }; +} + +var ReactFragment = { + // Wrap a keyed object in an opaque proxy that warns you if you access any + // of its properties. + create: function(object) { + if (__DEV__) { + 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]) { + warning( + didWarnForFragment(fragment), + 'Any use of a keyed object should be wrapped in ' + + 'React.addons.createFragment(object) before passed as a child.' + ); + 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); + } + } + } + } + return fragment; + } +}; + +module.exports = ReactFragment; diff --git a/src/addons/__tests__/ReactFragment-test.js b/src/addons/__tests__/ReactFragment-test.js new file mode 100644 index 0000000000..d7025f8029 --- /dev/null +++ b/src/addons/__tests__/ReactFragment-test.js @@ -0,0 +1,76 @@ +/** + * Copyright 2015, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @emails react-core + */ + +'use strict'; + +var React; +var ReactFragment; + +describe('ReactFragment', function() { + + beforeEach(function() { + React = require('React'); + ReactFragment = require('ReactFragment'); + }); + + it('should warn if a plain object is used as a child', function() { + spyOn(console, 'warn'); + var children = { + x: , + y: + }; +
{children}
; + expect(console.warn.calls.length).toBe(1); + expect(console.warn.calls[0].args[0]).toContain( + 'Any use of a keyed object' + ); + // Only warn once for the same set of children + var sameChildren = { + x: , + y: + }; +
{sameChildren}
; + expect(console.warn.calls.length).toBe(1); + }); + + it('should warn if a plain object even if it is deep', function() { + spyOn(console, 'warn'); + var children = { + x: , + y: , + z: + }; + var element =
{[children]}
; + expect(console.warn.calls.length).toBe(0); + var container = document.createElement('div'); + React.render(element, container); + expect(console.warn.calls.length).toBe(1); + expect(console.warn.calls[0].args[0]).toContain( + 'Any use of a keyed object' + ); + }); + + it('should warn if accessing any property on a fragment', function() { + spyOn(console, 'warn'); + var children = { + x: , + y: + }; + var frag = ReactFragment.create(children); + frag.x; + frag.y = 10; + expect(console.warn.calls.length).toBe(1); + expect(console.warn.calls[0].args[0]).toContain( + 'A ReactFragment is an opaque type' + ); + }); + +}); diff --git a/src/addons/transitions/ReactTransitionChildMapping.js b/src/addons/transitions/ReactTransitionChildMapping.js index c339196e4a..1daa1e1666 100644 --- a/src/addons/transitions/ReactTransitionChildMapping.js +++ b/src/addons/transitions/ReactTransitionChildMapping.js @@ -13,6 +13,7 @@ 'use strict'; var ReactChildren = require('ReactChildren'); +var ReactFragment = require('ReactFragment'); var ReactTransitionChildMapping = { /** @@ -23,9 +24,12 @@ var ReactTransitionChildMapping = { * @return {object} Mapping of key to child */ getChildMapping: function(children) { - return ReactChildren.map(children, function(child) { + if (!children) { + return children; + } + return ReactFragment.extract(ReactChildren.map(children, function(child) { return child; - }); + })); }, /** diff --git a/src/addons/transitions/ReactTransitionGroup.js b/src/addons/transitions/ReactTransitionGroup.js index 448f2f3554..bb99b66284 100644 --- a/src/addons/transitions/ReactTransitionGroup.js +++ b/src/addons/transitions/ReactTransitionGroup.js @@ -202,7 +202,7 @@ var ReactTransitionGroup = React.createClass({ render: function() { // TODO: we could get rid of the need for the wrapper node // by cloning a single child - var childrenToRender = {}; + var childrenToRender = []; for (var key in this.state.children) { var child = this.state.children[key]; if (child) { @@ -211,10 +211,10 @@ var ReactTransitionGroup = React.createClass({ // already been removed. In case you need this behavior you can provide // a childFactory function to wrap every child, even the ones that are // leaving. - childrenToRender[key] = cloneWithProps( + childrenToRender.push(cloneWithProps( this.props.childFactory(child), - {ref: key} - ); + {ref: key, key: key} + )); } } return React.createElement( diff --git a/src/browser/ReactWithAddons.js b/src/browser/ReactWithAddons.js index 6a70b210e7..6dfe9da083 100644 --- a/src/browser/ReactWithAddons.js +++ b/src/browser/ReactWithAddons.js @@ -23,6 +23,7 @@ var React = require('React'); var ReactComponentWithPureRenderMixin = require('ReactComponentWithPureRenderMixin'); var ReactCSSTransitionGroup = require('ReactCSSTransitionGroup'); +var ReactFragment = require('ReactFragment'); var ReactTransitionGroup = require('ReactTransitionGroup'); var ReactUpdates = require('ReactUpdates'); @@ -39,6 +40,7 @@ React.addons = { batchedUpdates: ReactUpdates.batchedUpdates, classSet: cx, cloneWithProps: cloneWithProps, + createFragment: ReactFragment.create, update: update }; diff --git a/src/browser/__tests__/ReactDOM-test.js b/src/browser/__tests__/ReactDOM-test.js index 31fc33a514..2feace23ad 100644 --- a/src/browser/__tests__/ReactDOM-test.js +++ b/src/browser/__tests__/ReactDOM-test.js @@ -75,37 +75,37 @@ describe('ReactDOM', function() { */ it("should purge the DOM cache when removing nodes", function() { var myDiv = ReactTestUtils.renderIntoDocument( -
{{ - theDog:
, - theBird:
- }}
+
+
, +
+
); // Warm the cache with theDog myDiv.setProps({ - children: { - theDog:
, - theBird:
- } + children: [ +
, +
+ ] }); // Remove theDog - this should purge the cache myDiv.setProps({ - children: { - theBird:
- } + children: [ +
+ ] }); // Now, put theDog back. It's now a different DOM node. myDiv.setProps({ - children: { - theDog:
, - theBird:
- } + children: [ +
, +
+ ] }); // Change the className of theDog. It will use the same element myDiv.setProps({ - children: { - theDog:
, - theBird:
- } + children: [ +
, +
+ ] }); var root = myDiv.getDOMNode(); var dog = root.childNodes[0]; diff --git a/src/classic/element/ReactElementValidator.js b/src/classic/element/ReactElementValidator.js index 52d69d5e6e..c1c7e30607 100644 --- a/src/classic/element/ReactElementValidator.js +++ b/src/classic/element/ReactElementValidator.js @@ -19,6 +19,7 @@ 'use strict'; var ReactElement = require('ReactElement'); +var ReactFragment = require('ReactFragment'); var ReactPropTypeLocations = require('ReactPropTypeLocations'); var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames'); var ReactCurrentOwner = require('ReactCurrentOwner'); @@ -175,21 +176,6 @@ function warnAndMonitorForKeyUse(warningID, message, element, parentType) { console.warn(message); } -/** - * Log that we're using an object map. We're considering deprecating this - * feature and replace it with proper Map and ImmutableMap data structures. - * - * @internal - */ -function monitorUseOfObjectMap() { - var currentName = getCurrentOwnerDisplayName() || ''; - if (ownerHasMonitoredObjectMap.hasOwnProperty(currentName)) { - return; - } - ownerHasMonitoredObjectMap[currentName] = true; - monitorCodeUse('react_object_map_children'); -} - /** * Ensure that every element either is passed in a static location, in an * array with an explicit keys property defined, or in an object literal @@ -213,19 +199,21 @@ function validateChildKeys(node, parentType) { } else if (node) { var iteratorFn = getIteratorFn(node); // Entry iterators provide implicit keys. - if (iteratorFn && iteratorFn !== node.entries) { - var iterator = iteratorFn.call(node); - var step; - while (!(step = iterator.next()).done) { - if (ReactElement.isValidElement(step.value)) { - validateExplicitKey(step.value, parentType); + if (iteratorFn) { + if (iteratorFn !== node.entries) { + var iterator = iteratorFn.call(node); + var step; + while (!(step = iterator.next()).done) { + if (ReactElement.isValidElement(step.value)) { + validateExplicitKey(step.value, parentType); + } } } } else if (typeof node === 'object') { - monitorUseOfObjectMap(); - for (var key in node) { - if (node.hasOwnProperty(key)) { - validatePropertyKey(key, node[key], parentType); + var fragment = ReactFragment.extractIfFragment(node); + for (var key in fragment) { + if (fragment.hasOwnProperty(key)) { + validatePropertyKey(key, fragment[key], parentType); } } } diff --git a/src/classic/element/__tests__/ReactElementValidator-test.js b/src/classic/element/__tests__/ReactElementValidator-test.js index 7bfc37ff35..a922ffbbbb 100644 --- a/src/classic/element/__tests__/ReactElementValidator-test.js +++ b/src/classic/element/__tests__/ReactElementValidator-test.js @@ -15,6 +15,7 @@ // classic JS without JSX. var React; +var ReactFragment; var ReactTestUtils; describe('ReactElementValidator', function() { @@ -24,12 +25,17 @@ describe('ReactElementValidator', function() { require('mock-modules').dumpCache(); React = require('React'); + ReactFragment = require('ReactFragment'); ReactTestUtils = require('ReactTestUtils'); ComponentClass = React.createClass({ render: function() { return React.createElement('div'); } }); }); + function frag(obj) { + return ReactFragment.create(obj); + } + it('warns for keys for arrays of elements in rest args', function() { spyOn(console, 'warn'); var Component = React.createFactory(ComponentClass); @@ -135,7 +141,7 @@ describe('ReactElementValidator', function() { spyOn(console, 'warn'); var Component = React.createFactory(ComponentClass); - Component(null, { 1: Component(), 2: Component() }); + Component(null, frag({ 1: Component(), 2: Component() })); expect(console.warn.argsForCall.length).toBe(1); expect(console.warn.argsForCall[0][0]).toContain( @@ -316,4 +322,12 @@ describe('ReactElementValidator', function() { expect(console.warn.calls.length).toBe(2); }); + it('should warn if a fragment is used without the wrapper', function () { + spyOn(console, 'warn'); + var child = React.createElement('span'); + React.createElement('div', null, { a: child, b: child }); + expect(console.warn.calls.length).toBe(1); + expect(console.warn.calls[0].args[0]).toContain('use of a keyed object'); + }); + }); diff --git a/src/core/__tests__/ReactIdentity-test.js b/src/core/__tests__/ReactIdentity-test.js index 366920db19..c93ecdd450 100644 --- a/src/core/__tests__/ReactIdentity-test.js +++ b/src/core/__tests__/ReactIdentity-test.js @@ -12,6 +12,7 @@ 'use strict'; var React; +var ReactFragment; var ReactTestUtils; var reactComponentExpect; var ReactMount; @@ -21,6 +22,7 @@ describe('ReactIdentity', function() { beforeEach(function() { require('mock-modules').dumpCache(); React = require('React'); + ReactFragment = require('ReactFragment'); ReactTestUtils = require('ReactTestUtils'); reactComponentExpect = require('reactComponentExpect'); ReactMount = require('ReactMount'); @@ -35,13 +37,17 @@ describe('ReactIdentity', function() { expect(actual[1]).toEqual(expected[1]); } + function frag(obj) { + return ReactFragment.create(obj); + } + it('should allow keyed objects to express identity', function() { var instance =
- {{ + {frag({ first:
, second:
- }} + })}
; instance = React.render(instance, document.createElement('div')); @@ -106,7 +112,7 @@ describe('ReactIdentity', function() { var map = {}; map[key] = span2; - return
{[span1, map]}
; + return
{[span1, frag(map)]}
; } }); diff --git a/src/core/__tests__/ReactMultiChildReconcile-test.js b/src/core/__tests__/ReactMultiChildReconcile-test.js index 4948e8f39c..ebbe661d83 100644 --- a/src/core/__tests__/ReactMultiChildReconcile-test.js +++ b/src/core/__tests__/ReactMultiChildReconcile-test.js @@ -39,7 +39,7 @@ var stripEmptyValues = function(obj) { * here. This relies on an implementation detail of the rendering system. */ var getOriginalKey = function(childName) { - var match = childName.match(/^\.\$([^.]+)\:0$/); + var match = childName.match(/^\.\$([^.]+)$/); expect(match).not.toBeNull(); return match[1]; }; @@ -107,13 +107,14 @@ var FriendsStatusDisplay = React.createClass({ return res; }, render: function() { - var children = null; + var children = []; var key; for (key in this.props.usernameToStatus) { var status = this.props.usernameToStatus[key]; - children = children || {}; - children[key] = !status ? null : - ; + children.push( + !status ? null : + + ); } return (
diff --git a/src/core/__tests__/ReactMultiChildText-test.js b/src/core/__tests__/ReactMultiChildText-test.js index 0efdc812fc..764dd1b746 100644 --- a/src/core/__tests__/ReactMultiChildText-test.js +++ b/src/core/__tests__/ReactMultiChildText-test.js @@ -16,10 +16,13 @@ require('mock-modules'); var React = require('React'); +var ReactFragment = require('ReactFragment'); var ReactTestUtils = require('ReactTestUtils'); var reactComponentExpect = require('reactComponentExpect'); +var frag = ReactFragment.create; + // Helpers var testAllPermutations = function(testCases) { for (var i = 0; i < testCases.length; i += 2) { @@ -182,18 +185,18 @@ describe('ReactMultiChildText', function() { ['', 'foo', [true,
, 1.2, ''], 'foo'], ['', 'foo',
, '1.2', '', 'foo'], // values inside objects - [{a: true}, {a: true}], [], - [{a: 1.2}, {a: 1.2}], ['1.2', '1.2'], - [{a: ''}, {a: ''}], ['', ''], - [{a: 'foo'}, {a: 'foo'}], ['foo', 'foo'], - [{a:
}, {a:
}], [
,
], + [frag({a: true}), frag({a: true})], [], + [frag({a: 1.2}), frag({a: 1.2})], ['1.2', '1.2'], + [frag({a: ''}), frag({a: ''})], ['', ''], + [frag({a: 'foo'}), frag({a: 'foo'})], ['foo', 'foo'], + [frag({a:
}), frag({a:
})], [
,
], - [{a: true, b: 1.2, c:
}, '', 'foo'], ['1.2',
, '', 'foo'], - [1.2, '', {a:
, b: 'foo', c: true}], ['1.2', '',
, 'foo'], - ['', {a: 'foo', b:
, c: true}, 1.2], ['', 'foo',
, '1.2'], + [frag({a: true, b: 1.2, c:
}), '', 'foo'], ['1.2',
, '', 'foo'], + [1.2, '', frag({a:
, b: 'foo', c: true})], ['1.2', '',
, 'foo'], + ['', frag({a: 'foo', b:
, c: true}), 1.2], ['', 'foo',
, '1.2'], - [true, {a: 1.2, b: '', c:
, d: 'foo'}, true, 1.2], ['1.2', '',
, 'foo', '1.2'], - ['', 'foo', {a: true, b:
, c: 1.2, d: ''}, 'foo'], ['', 'foo',
, '1.2', '', 'foo'], + [true, frag({a: 1.2, b: '', c:
, d: 'foo'}), true, 1.2], ['1.2', '',
, 'foo', '1.2'], + ['', 'foo', frag({a: true, b:
, c: 1.2, d: ''}), 'foo'], ['', 'foo',
, '1.2', '', 'foo'], // values inside elements [
{true}{1.2}{
}
, '', 'foo'], [
, '', 'foo'], diff --git a/src/modern/element/__tests__/ReactJSXElementValidator-test.js b/src/modern/element/__tests__/ReactJSXElementValidator-test.js index c1c05fb946..d664a24d27 100644 --- a/src/modern/element/__tests__/ReactJSXElementValidator-test.js +++ b/src/modern/element/__tests__/ReactJSXElementValidator-test.js @@ -15,6 +15,7 @@ // of dynamic errors when using JSX with Flow. var React; +var ReactFragment; var ReactTestUtils; describe('ReactJSXElementValidator', function() { @@ -24,6 +25,7 @@ describe('ReactJSXElementValidator', function() { require('mock-modules').dumpCache(); React = require('React'); + ReactFragment = require('ReactFragment'); ReactTestUtils = require('ReactTestUtils'); Component = class { @@ -31,6 +33,10 @@ describe('ReactJSXElementValidator', function() { }; }); + function frag(obj) { + return ReactFragment.create(obj); + } + it('warns for keys for arrays of elements in children position', function() { spyOn(console, 'warn'); @@ -128,7 +134,7 @@ describe('ReactJSXElementValidator', function() { it('warns for numeric keys on objects as children', function() { spyOn(console, 'warn'); - {{ 1: , 2: }}; + {frag({ 1: , 2: })}; expect(console.warn.argsForCall.length).toBe(1); expect(console.warn.argsForCall[0][0]).toContain( diff --git a/src/utils/ReactChildren.js b/src/utils/ReactChildren.js index 24edcd42e6..4109557dde 100644 --- a/src/utils/ReactChildren.js +++ b/src/utils/ReactChildren.js @@ -12,6 +12,7 @@ 'use strict'; var PooledClass = require('PooledClass'); +var ReactFragment = require('ReactFragment'); var traverseAllChildren = require('traverseAllChildren'); var warning = require('warning'); @@ -121,7 +122,7 @@ function mapChildren(children, func, context) { var traverseContext = MapBookKeeping.getPooled(mapResult, func, context); traverseAllChildren(children, mapSingleChildIntoContext, traverseContext); MapBookKeeping.release(traverseContext); - return mapResult; + return ReactFragment.create(mapResult); } function forEachSingleChildDummy(traverseContext, child, name, i) { diff --git a/src/utils/__tests__/ReactChildren-test.js b/src/utils/__tests__/ReactChildren-test.js index 550af41eb7..8a271de6be 100644 --- a/src/utils/__tests__/ReactChildren-test.js +++ b/src/utils/__tests__/ReactChildren-test.js @@ -14,12 +14,31 @@ describe('ReactChildren', function() { var ReactChildren; var React; + var ReactFragment; beforeEach(function() { ReactChildren = require('ReactChildren'); React = require('React'); + ReactFragment = require('ReactFragment'); }); + function frag(obj) { + return ReactFragment.create(obj); + } + + 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) { @@ -37,7 +56,7 @@ describe('ReactChildren', function() { callback.reset(); var mappedChildren = ReactChildren.map(instance.props.children, callback); expect(callback).toHaveBeenCalledWith(simpleKid, 0); - expect(mappedChildren[Object.keys(mappedChildren)[0]]).toBe(simpleKid); + expect(nthChild(mappedChildren, 0)).toBe(simpleKid); }); it('should treat single arrayless child as being in array', function() { @@ -52,7 +71,7 @@ describe('ReactChildren', function() { callback.reset(); var mappedChildren = ReactChildren.map(instance.props.children, callback); expect(callback).toHaveBeenCalledWith(simpleKid, 0); - expect(mappedChildren[Object.keys(mappedChildren)[0]]).toBe(simpleKid); + expect(nthChild(mappedChildren, 0)).toBe(simpleKid); }); it('should treat single child in array as expected', function() { @@ -67,7 +86,7 @@ describe('ReactChildren', function() { callback.reset(); var mappedChildren = ReactChildren.map(instance.props.children, callback); expect(callback).toHaveBeenCalledWith(simpleKid, 0); - expect(mappedChildren[Object.keys(mappedChildren)[0]]).toBe(simpleKid); + expect(nthChild(mappedChildren, 0)).toBe(simpleKid); }); it('should pass key to returned component', function() { @@ -80,11 +99,10 @@ describe('ReactChildren', function() { var instance =
{simpleKid}
; var mappedChildren = ReactChildren.map(instance.props.children, mapFn); - var mappedKeys = Object.keys(mappedChildren); - expect(mappedKeys.length).toBe(1); - expect(mappedChildren[mappedKeys[0]]).not.toBe(simpleKid); - expect(mappedChildren[mappedKeys[0]].props.children).toBe(simpleKid); - expect(mappedKeys[0]).toBe('.$simple'); + 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'); }); it('should invoke callback with the right context', function() { @@ -94,7 +112,8 @@ describe('ReactChildren', function() { return this; }; - var scopeTester = {}; + // TODO: Use an object to test, after non-object fragments has fully landed. + var scopeTester = 'scope tester'; var simpleKid = ; var instance =
{simpleKid}
; @@ -104,9 +123,8 @@ describe('ReactChildren', function() { var mappedChildren = ReactChildren.map(instance.props.children, callback, scopeTester); - var mappedKeys = Object.keys(mappedChildren); - expect(mappedKeys.length).toBe(1); - expect(mappedChildren[mappedKeys[0]]).toBe(scopeTester); + expect(ReactChildren.count(mappedChildren)).toBe(1); + expect(nthChild(mappedChildren, 0)).toBe(scopeTester); }); it('should be called for each child', function() { @@ -149,28 +167,33 @@ describe('ReactChildren', function() { var mappedChildren = ReactChildren.map(instance.props.children, callback); - var mappedKeys = Object.keys(mappedChildren); expect(callback.calls.length).toBe(5); - expect(mappedKeys.length).toBe(5); + expect(ReactChildren.count(mappedChildren)).toBe(5); // Keys default to indices. - expect(mappedKeys).toEqual( + expect([ + keyOfNthChild(mappedChildren, 0), + keyOfNthChild(mappedChildren, 1), + keyOfNthChild(mappedChildren, 2), + keyOfNthChild(mappedChildren, 3), + keyOfNthChild(mappedChildren, 4) + ]).toEqual( ['.$keyZero', '.1', '.$keyTwo', '.3', '.$keyFour'] ); expect(callback).toHaveBeenCalledWith(zero, 0); - expect(mappedChildren[mappedKeys[0]]).toBe(zeroMapped); + expect(nthChild(mappedChildren, 0)).toBe(zeroMapped); expect(callback).toHaveBeenCalledWith(one, 1); - expect(mappedChildren[mappedKeys[1]]).toBe(oneMapped); + expect(nthChild(mappedChildren, 1)).toBe(oneMapped); expect(callback).toHaveBeenCalledWith(two, 2); - expect(mappedChildren[mappedKeys[2]]).toBe(twoMapped); + expect(nthChild(mappedChildren, 2)).toBe(twoMapped); expect(callback).toHaveBeenCalledWith(three, 3); - expect(mappedChildren[mappedKeys[3]]).toBe(threeMapped); + expect(nthChild(mappedChildren, 3)).toBe(threeMapped); expect(callback).toHaveBeenCalledWith(four, 4); - expect(mappedChildren[mappedKeys[4]]).toBe(fourMapped); + expect(nthChild(mappedChildren, 4)).toBe(fourMapped); }); @@ -204,11 +227,11 @@ describe('ReactChildren', function() { var instance = (
{ - [{ + [frag({ firstHalfKey: [zero, one, two], secondHalfKey: [three, four], keyFive: five - }] + })] }
); @@ -222,11 +245,17 @@ describe('ReactChildren', function() { callback.reset(); var mappedChildren = ReactChildren.map(instance.props.children, callback); - var mappedKeys = Object.keys(mappedChildren); expect(callback.calls.length).toBe(6); - expect(mappedKeys.length).toBe(6); + expect(ReactChildren.count(mappedChildren)).toBe(6); // Keys default to indices. - expect(mappedKeys).toEqual([ + expect([ + keyOfNthChild(mappedChildren, 0), + keyOfNthChild(mappedChildren, 1), + keyOfNthChild(mappedChildren, 2), + keyOfNthChild(mappedChildren, 3), + keyOfNthChild(mappedChildren, 4), + keyOfNthChild(mappedChildren, 5) + ]).toEqual([ '.0:$firstHalfKey:0:$keyZero', '.0:$firstHalfKey:0:1', '.0:$firstHalfKey:0:$keyTwo', @@ -236,22 +265,22 @@ describe('ReactChildren', function() { ]); expect(callback).toHaveBeenCalledWith(zero, 0); - expect(mappedChildren[mappedKeys[0]]).toBe(zeroMapped); + expect(nthChild(mappedChildren, 0)).toBe(zeroMapped); expect(callback).toHaveBeenCalledWith(one, 1); - expect(mappedChildren[mappedKeys[1]]).toBe(oneMapped); + expect(nthChild(mappedChildren, 1)).toBe(oneMapped); expect(callback).toHaveBeenCalledWith(two, 2); - expect(mappedChildren[mappedKeys[2]]).toBe(twoMapped); + expect(nthChild(mappedChildren, 2)).toBe(twoMapped); expect(callback).toHaveBeenCalledWith(three, 3); - expect(mappedChildren[mappedKeys[3]]).toBe(threeMapped); + expect(nthChild(mappedChildren, 3)).toBe(threeMapped); expect(callback).toHaveBeenCalledWith(four, 4); - expect(mappedChildren[mappedKeys[4]]).toBe(fourMapped); + expect(nthChild(mappedChildren, 4)).toBe(fourMapped); expect(callback).toHaveBeenCalledWith(five, 5); - expect(mappedChildren[mappedKeys[5]]).toBe(fiveMapped); + expect(nthChild(mappedChildren, 5)).toBe(fiveMapped); }); it('should retain key across two mappings', function() { @@ -325,7 +354,8 @@ describe('ReactChildren', function() { var mapped = ReactChildren.map(instance.props.children, mapFn); expect(console.warn.calls.length).toEqual(1); - expect(mapped).toEqual({'.$something': zero}); + expect(nthChild(mapped, 0)).toBe(zero); + expect(keyOfNthChild(mapped, 0)).toBe('.$something'); }); it('should return 0 for null children', function() { @@ -380,14 +410,49 @@ describe('ReactChildren', function() { var instance = (
{ - [{ + [frag({ firstHalfKey: [zero, one, two], secondHalfKey: [three, four], keyFive: five - }] + })] }
); var numberOfChildren = ReactChildren.count(instance.props.children); expect(numberOfChildren).toBe(6); }); + + it('should warn if a fragment is used without the wrapper', function () { + spyOn(console, 'warn'); + var child = React.createElement('span'); + ReactChildren.forEach({ a: child, b: child}, function(c) { + expect(c).toBe(child); + }); + expect(console.warn.calls.length).toBe(1); + expect(console.warn.calls[0].args[0]).toContain('use of a keyed object'); + }); + + it('should warn if a fragment is accessed', function () { + spyOn(console, 'warn'); + var child = React.createElement('span'); + var frag = ReactChildren.map([ child, child ], function(c) { + return c; + }); + for (var key in frag) { + frag[key]; + break; + } + expect(console.warn.calls.length).toBe(1); + expect(console.warn.calls[0].args[0]).toContain('is an opaque type'); + + var frag2 = ReactChildren.map([ child, child ], function(c) { + return c; + }); + for (var key in frag2) { + frag2[key] = 123; + break; + } + expect(console.warn.calls.length).toBe(2); + expect(console.warn.calls[1].args[0]).toContain('is an immutable opaque'); + }); + }); diff --git a/src/utils/__tests__/onlyChild-test.js b/src/utils/__tests__/onlyChild-test.js index 835bf794d9..29a7c77226 100644 --- a/src/utils/__tests__/onlyChild-test.js +++ b/src/utils/__tests__/onlyChild-test.js @@ -14,11 +14,13 @@ describe('onlyChild', function() { var React; + var ReactFragment; var onlyChild; var WrapComponent; beforeEach(function() { React = require('React'); + ReactFragment = require('ReactFragment'); onlyChild = require('onlyChild'); WrapComponent = React.createClass({ render: function() { @@ -64,7 +66,7 @@ describe('onlyChild', function() { expect(function() { var instance = - {{oneThing: }} + {ReactFragment.create({oneThing: })} ; onlyChild(instance.props.children); }).toThrow(); diff --git a/src/utils/__tests__/sliceChildren-test.js b/src/utils/__tests__/sliceChildren-test.js index 47afb17d86..065da44697 100644 --- a/src/utils/__tests__/sliceChildren-test.js +++ b/src/utils/__tests__/sliceChildren-test.js @@ -14,6 +14,7 @@ describe('sliceChildren', function() { var React; + var ReactFragment; var ReactTestUtils; var sliceChildren; @@ -23,6 +24,7 @@ describe('sliceChildren', function() { beforeEach(function() { React = require('React'); + ReactFragment = require('ReactFragment'); ReactTestUtils = require('ReactTestUtils'); sliceChildren = require('sliceChildren'); @@ -52,6 +54,11 @@ describe('sliceChildren', function() { return rendered.props.children; } + 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 = [
, @@ -59,7 +66,7 @@ describe('sliceChildren', function() {
]; var children = renderAndSlice(fullSet, 0); - expect(children).toEqual({ + testKeyValuePairs(children, { '.$A': fullSet[0], '.$B': fullSet[1], '.$C': fullSet[2] @@ -73,7 +80,7 @@ describe('sliceChildren', function() {
]; var children = renderAndSlice(fullSet, 1); - expect(children).toEqual({ + testKeyValuePairs(children, { '.$B': fullSet[1], '.$C': fullSet[2] }); @@ -87,7 +94,7 @@ describe('sliceChildren', function() {
]; var children = renderAndSlice(fullSet, 1, 2); - expect(children).toEqual({ + testKeyValuePairs(children, { '.$B': fullSet[1] }); }); @@ -103,7 +110,7 @@ describe('sliceChildren', function() { .expectRenderedChild() .instance(); - expect(rendered.props.children).toEqual({ + testKeyValuePairs(rendered.props.children, { '.1': b }); }); diff --git a/src/utils/__tests__/traverseAllChildren-test.js b/src/utils/__tests__/traverseAllChildren-test.js index c32e07c66d..91b51f0292 100644 --- a/src/utils/__tests__/traverseAllChildren-test.js +++ b/src/utils/__tests__/traverseAllChildren-test.js @@ -14,11 +14,16 @@ describe('traverseAllChildren', function() { var traverseAllChildren; var React; + var ReactFragment; beforeEach(function() { traverseAllChildren = require('traverseAllChildren'); React = require('React'); + ReactFragment = require('ReactFragment'); }); + function frag(obj) { + return ReactFragment.create(obj); + } it('should support identity for simple', function() { var traverseContext = []; @@ -139,8 +144,8 @@ describe('traverseAllChildren', function() { var instance = (
{div} - {[{span}]} - {{a: a}} + {[frag({span})]} + {frag({a: a})} {'string'} {1234} {true} @@ -206,11 +211,11 @@ describe('traverseAllChildren', function() { var instance = (
{ - [{ + [frag({ firstHalfKey: [zero, one, two], secondHalfKey: [three, four], keyFive: five - }] + })] }
); diff --git a/src/utils/sliceChildren.js b/src/utils/sliceChildren.js index fee13fc6c0..22a01dea14 100644 --- a/src/utils/sliceChildren.js +++ b/src/utils/sliceChildren.js @@ -11,6 +11,8 @@ 'use strict'; +var ReactFragment = require('ReactFragment'); + var flattenChildren = require('flattenChildren'); /** @@ -43,7 +45,7 @@ function sliceChildren(children, start, end) { break; } } - return slicedChildren; + return ReactFragment.create(slicedChildren); } module.exports = sliceChildren; diff --git a/src/utils/traverseAllChildren.js b/src/utils/traverseAllChildren.js index d0a31fd489..da8fd53bb9 100644 --- a/src/utils/traverseAllChildren.js +++ b/src/utils/traverseAllChildren.js @@ -12,6 +12,7 @@ 'use strict'; var ReactElement = require('ReactElement'); +var ReactFragment = require('ReactFragment'); var ReactInstanceHandles = require('ReactInstanceHandles'); var getIteratorFn = require('getIteratorFn'); @@ -184,9 +185,10 @@ function traverseAllChildrenImpl( 'traverseAllChildren(...): Encountered an invalid child; DOM ' + 'elements are not valid children of React components.' ); - for (var key in children) { - if (children.hasOwnProperty(key)) { - child = children[key]; + var fragment = ReactFragment.extract(children); + for (var key in fragment) { + if (fragment.hasOwnProperty(key)) { + child = fragment[key]; nextName = ( (nameSoFar !== '' ? nameSoFar + SUBSEPARATOR : SEPARATOR) + wrapUserProvidedKey(key) + SUBSEPARATOR +