Skip to content

Commit 56f5115

Browse files
sebmarkbagesebmarkbage
committedFeb 9, 2015
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.
·
v19.1.00.14.10
1 parent 44634c0 commit 56f5115

19 files changed

+468
-120
lines changed
 

‎src/addons/ReactFragment.js

+164
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
/**
2+
* Copyright 2015, Facebook, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree. An additional grant
7+
* of patent rights can be found in the PATENTS file in the same directory.
8+
*
9+
* @providesModule ReactFragment
10+
*/
11+
12+
'use strict';
13+
14+
var ReactElement = require('ReactElement');
15+
16+
var warning = require('warning');
17+
18+
/**
19+
* We used to allow keyed objects to serve as a collection of ReactElements,
20+
* or nested sets. This allowed us a way to explicitly key a set a fragment of
21+
* components. This is now being replaced with an opaque data structure.
22+
* The upgrade path is to call React.addons.createFragment({ key: value }) to
23+
* create a keyed fragment. The resulting data structure is opaque, for now.
24+
*/
25+
26+
if (__DEV__) {
27+
var fragmentKey = '_reactFragment';
28+
var didWarnKey = '_reactDidWarn';
29+
var canWarnForReactFragment = false;
30+
31+
try {
32+
// Feature test. Don't even try to issue this warning if we can't use
33+
// enumerable: false.
34+
35+
var dummy = function() {
36+
return 1;
37+
};
38+
39+
Object.defineProperty(
40+
{},
41+
fragmentKey,
42+
{ enumerable: false, value: true }
43+
);
44+
45+
Object.defineProperty(
46+
{},
47+
'key',
48+
{ enumerable: true, get: dummy }
49+
);
50+
51+
canWarnForReactFragment = true;
52+
} catch (x) { }
53+
54+
var proxyPropertyAccessWithWarning = function(obj, key) {
55+
Object.defineProperty(obj, key, {
56+
enumerable: true,
57+
get: function() {
58+
warning(
59+
this[didWarnKey],
60+
'A ReactFragment is an opaque type. Accessing any of its ' +
61+
'properties is deprecated. Pass it to one of the React.Children ' +
62+
'helpers.'
63+
);
64+
this[didWarnKey] = true;
65+
return this[fragmentKey][key];
66+
},
67+
set: function(value) {
68+
warning(
69+
this[didWarnKey],
70+
'A ReactFragment is an immutable opaque type. Mutating its ' +
71+
'properties is deprecated.'
72+
);
73+
this[didWarnKey] = true;
74+
this[fragmentKey][key] = value;
75+
}
76+
});
77+
};
78+
79+
var issuedWarnings = {};
80+
81+
var didWarnForFragment = function(fragment) {
82+
// We use the keys and the type of the value as a heuristic to dedupe the
83+
// warning to avoid spamming too much.
84+
var fragmentCacheKey = '';
85+
for (var key in fragment) {
86+
fragmentCacheKey += key + ':' + (typeof fragment[key]) + ',';
87+
}
88+
var alreadyWarnedOnce = !!issuedWarnings[fragmentCacheKey];
89+
issuedWarnings[fragmentCacheKey] = true;
90+
return alreadyWarnedOnce;
91+
};
92+
}
93+
94+
var ReactFragment = {
95+
// Wrap a keyed object in an opaque proxy that warns you if you access any
96+
// of its properties.
97+
create: function(object) {
98+
if (__DEV__) {
99+
if (canWarnForReactFragment) {
100+
var proxy = {};
101+
Object.defineProperty(proxy, fragmentKey, {
102+
enumerable: false,
103+
value: object
104+
});
105+
Object.defineProperty(proxy, didWarnKey, {
106+
writable: true,
107+
enumerable: false,
108+
value: false
109+
});
110+
for (var key in object) {
111+
proxyPropertyAccessWithWarning(proxy, key);
112+
}
113+
Object.preventExtensions(proxy);
114+
return proxy;
115+
}
116+
}
117+
return object;
118+
},
119+
// Extract the original keyed object from the fragment opaque type. Warn if
120+
// a plain object is passed here.
121+
extract: function(fragment) {
122+
if (__DEV__) {
123+
if (canWarnForReactFragment) {
124+
if (!fragment[fragmentKey]) {
125+
warning(
126+
didWarnForFragment(fragment),
127+
'Any use of a keyed object should be wrapped in ' +
128+
'React.addons.createFragment(object) before passed as a child.'
129+
);
130+
return fragment;
131+
}
132+
return fragment[fragmentKey];
133+
}
134+
}
135+
return fragment;
136+
},
137+
// Check if this is a fragment and if so, extract the keyed object. If it
138+
// is a fragment-like object, warn that it should be wrapped. Ignore if we
139+
// can't determine what kind of object this is.
140+
extractIfFragment: function(fragment) {
141+
if (__DEV__) {
142+
if (canWarnForReactFragment) {
143+
// If it is the opaque type, return the keyed object.
144+
if (fragment[fragmentKey]) {
145+
return fragment[fragmentKey];
146+
}
147+
// Otherwise, check each property if it has an element, if it does
148+
// it is probably meant as a fragment, so we can warn early. Defer,
149+
// the warning to extract.
150+
for (var key in fragment) {
151+
if (fragment.hasOwnProperty(key) &&
152+
ReactElement.isValidElement(fragment[key])) {
153+
// This looks like a fragment object, we should provide an
154+
// early warning.
155+
return ReactFragment.extract(fragment);
156+
}
157+
}
158+
}
159+
}
160+
return fragment;
161+
}
162+
};
163+
164+
module.exports = ReactFragment;

‎src/addons/__tests__/ReactFragment-test.js

+76
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/**
2+
* Copyright 2015, Facebook, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree. An additional grant
7+
* of patent rights can be found in the PATENTS file in the same directory.
8+
*
9+
* @emails react-core
10+
*/
11+
12+
'use strict';
13+
14+
var React;
15+
var ReactFragment;
16+
17+
describe('ReactFragment', function() {
18+
19+
beforeEach(function() {
20+
React = require('React');
21+
ReactFragment = require('ReactFragment');
22+
});
23+
24+
it('should warn if a plain object is used as a child', function() {
25+
spyOn(console, 'warn');
26+
var children = {
27+
x: <span />,
28+
y: <span />
29+
};
30+
<div>{children}</div>;
31+
expect(console.warn.calls.length).toBe(1);
32+
expect(console.warn.calls[0].args[0]).toContain(
33+
'Any use of a keyed object'
34+
);
35+
// Only warn once for the same set of children
36+
var sameChildren = {
37+
x: <span />,
38+
y: <span />
39+
};
40+
<div>{sameChildren}</div>;
41+
expect(console.warn.calls.length).toBe(1);
42+
});
43+
44+
it('should warn if a plain object even if it is deep', function() {
45+
spyOn(console, 'warn');
46+
var children = {
47+
x: <span />,
48+
y: <span />,
49+
z: <span />
50+
};
51+
var element = <div>{[children]}</div>;
52+
expect(console.warn.calls.length).toBe(0);
53+
var container = document.createElement('div');
54+
React.render(element, container);
55+
expect(console.warn.calls.length).toBe(1);
56+
expect(console.warn.calls[0].args[0]).toContain(
57+
'Any use of a keyed object'
58+
);
59+
});
60+
61+
it('should warn if accessing any property on a fragment', function() {
62+
spyOn(console, 'warn');
63+
var children = {
64+
x: <span />,
65+
y: <span />
66+
};
67+
var frag = ReactFragment.create(children);
68+
frag.x;
69+
frag.y = 10;
70+
expect(console.warn.calls.length).toBe(1);
71+
expect(console.warn.calls[0].args[0]).toContain(
72+
'A ReactFragment is an opaque type'
73+
);
74+
});
75+
76+
});

‎src/addons/transitions/ReactTransitionChildMapping.js

+6-2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
'use strict';
1414

1515
var ReactChildren = require('ReactChildren');
16+
var ReactFragment = require('ReactFragment');
1617

1718
var ReactTransitionChildMapping = {
1819
/**
@@ -23,9 +24,12 @@ var ReactTransitionChildMapping = {
2324
* @return {object} Mapping of key to child
2425
*/
2526
getChildMapping: function(children) {
26-
return ReactChildren.map(children, function(child) {
27+
if (!children) {
28+
return children;
29+
}
30+
return ReactFragment.extract(ReactChildren.map(children, function(child) {
2731
return child;
28-
});
32+
}));
2933
},
3034

3135
/**

‎src/addons/transitions/ReactTransitionGroup.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ var ReactTransitionGroup = React.createClass({
202202
render: function() {
203203
// TODO: we could get rid of the need for the wrapper node
204204
// by cloning a single child
205-
var childrenToRender = {};
205+
var childrenToRender = [];
206206
for (var key in this.state.children) {
207207
var child = this.state.children[key];
208208
if (child) {
@@ -211,10 +211,10 @@ var ReactTransitionGroup = React.createClass({
211211
// already been removed. In case you need this behavior you can provide
212212
// a childFactory function to wrap every child, even the ones that are
213213
// leaving.
214-
childrenToRender[key] = cloneWithProps(
214+
childrenToRender.push(cloneWithProps(
215215
this.props.childFactory(child),
216-
{ref: key}
217-
);
216+
{ref: key, key: key}
217+
));
218218
}
219219
}
220220
return React.createElement(

‎src/browser/ReactWithAddons.js

+2
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ var React = require('React');
2323
var ReactComponentWithPureRenderMixin =
2424
require('ReactComponentWithPureRenderMixin');
2525
var ReactCSSTransitionGroup = require('ReactCSSTransitionGroup');
26+
var ReactFragment = require('ReactFragment');
2627
var ReactTransitionGroup = require('ReactTransitionGroup');
2728
var ReactUpdates = require('ReactUpdates');
2829

@@ -39,6 +40,7 @@ React.addons = {
3940
batchedUpdates: ReactUpdates.batchedUpdates,
4041
classSet: cx,
4142
cloneWithProps: cloneWithProps,
43+
createFragment: ReactFragment.create,
4244
update: update
4345
};
4446

‎src/browser/__tests__/ReactDOM-test.js

+19-19
Original file line numberDiff line numberDiff line change
@@ -75,37 +75,37 @@ describe('ReactDOM', function() {
7575
*/
7676
it("should purge the DOM cache when removing nodes", function() {
7777
var myDiv = ReactTestUtils.renderIntoDocument(
78-
<div>{{
79-
theDog: <div className="dog" />,
80-
theBird: <div className="bird" />
81-
}}</div>
78+
<div>
79+
<div key="theDog" className="dog" />,
80+
<div key="theBird" className="bird" />
81+
</div>
8282
);
8383
// Warm the cache with theDog
8484
myDiv.setProps({
85-
children: {
86-
theDog: <div className="dogbeforedelete" />,
87-
theBird: <div className="bird" />
88-
}
85+
children: [
86+
<div key="theDog" className="dogbeforedelete" />,
87+
<div key="theBird" className="bird" />
88+
]
8989
});
9090
// Remove theDog - this should purge the cache
9191
myDiv.setProps({
92-
children: {
93-
theBird: <div className="bird" />
94-
}
92+
children: [
93+
<div key="theBird" className="bird" />
94+
]
9595
});
9696
// Now, put theDog back. It's now a different DOM node.
9797
myDiv.setProps({
98-
children: {
99-
theDog: <div className="dog" />,
100-
theBird: <div className="bird" />
101-
}
98+
children: [
99+
<div key="theDog" className="dog" />,
100+
<div key="theBird" className="bird" />
101+
]
102102
});
103103
// Change the className of theDog. It will use the same element
104104
myDiv.setProps({
105-
children: {
106-
theDog: <div className="bigdog" />,
107-
theBird: <div className="bird" />
108-
}
105+
children: [
106+
<div key="theDog" className="bigdog" />,
107+
<div key="theBird" className="bird" />
108+
]
109109
});
110110
var root = myDiv.getDOMNode();
111111
var dog = root.childNodes[0];

‎src/classic/element/ReactElementValidator.js

+13-25
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
'use strict';
2020

2121
var ReactElement = require('ReactElement');
22+
var ReactFragment = require('ReactFragment');
2223
var ReactPropTypeLocations = require('ReactPropTypeLocations');
2324
var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames');
2425
var ReactCurrentOwner = require('ReactCurrentOwner');
@@ -175,21 +176,6 @@ function warnAndMonitorForKeyUse(warningID, message, element, parentType) {
175176
console.warn(message);
176177
}
177178

178-
/**
179-
* Log that we're using an object map. We're considering deprecating this
180-
* feature and replace it with proper Map and ImmutableMap data structures.
181-
*
182-
* @internal
183-
*/
184-
function monitorUseOfObjectMap() {
185-
var currentName = getCurrentOwnerDisplayName() || '';
186-
if (ownerHasMonitoredObjectMap.hasOwnProperty(currentName)) {
187-
return;
188-
}
189-
ownerHasMonitoredObjectMap[currentName] = true;
190-
monitorCodeUse('react_object_map_children');
191-
}
192-
193179
/**
194180
* Ensure that every element either is passed in a static location, in an
195181
* array with an explicit keys property defined, or in an object literal
@@ -213,19 +199,21 @@ function validateChildKeys(node, parentType) {
213199
} else if (node) {
214200
var iteratorFn = getIteratorFn(node);
215201
// Entry iterators provide implicit keys.
216-
if (iteratorFn && iteratorFn !== node.entries) {
217-
var iterator = iteratorFn.call(node);
218-
var step;
219-
while (!(step = iterator.next()).done) {
220-
if (ReactElement.isValidElement(step.value)) {
221-
validateExplicitKey(step.value, parentType);
202+
if (iteratorFn) {
203+
if (iteratorFn !== node.entries) {
204+
var iterator = iteratorFn.call(node);
205+
var step;
206+
while (!(step = iterator.next()).done) {
207+
if (ReactElement.isValidElement(step.value)) {
208+
validateExplicitKey(step.value, parentType);
209+
}
222210
}
223211
}
224212
} else if (typeof node === 'object') {
225-
monitorUseOfObjectMap();
226-
for (var key in node) {
227-
if (node.hasOwnProperty(key)) {
228-
validatePropertyKey(key, node[key], parentType);
213+
var fragment = ReactFragment.extractIfFragment(node);
214+
for (var key in fragment) {
215+
if (fragment.hasOwnProperty(key)) {
216+
validatePropertyKey(key, fragment[key], parentType);
229217
}
230218
}
231219
}

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

+15-1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
// classic JS without JSX.
1616

1717
var React;
18+
var ReactFragment;
1819
var ReactTestUtils;
1920

2021
describe('ReactElementValidator', function() {
@@ -24,12 +25,17 @@ describe('ReactElementValidator', function() {
2425
require('mock-modules').dumpCache();
2526

2627
React = require('React');
28+
ReactFragment = require('ReactFragment');
2729
ReactTestUtils = require('ReactTestUtils');
2830
ComponentClass = React.createClass({
2931
render: function() { return React.createElement('div'); }
3032
});
3133
});
3234

35+
function frag(obj) {
36+
return ReactFragment.create(obj);
37+
}
38+
3339
it('warns for keys for arrays of elements in rest args', function() {
3440
spyOn(console, 'warn');
3541
var Component = React.createFactory(ComponentClass);
@@ -135,7 +141,7 @@ describe('ReactElementValidator', function() {
135141
spyOn(console, 'warn');
136142
var Component = React.createFactory(ComponentClass);
137143

138-
Component(null, { 1: Component(), 2: Component() });
144+
Component(null, frag({ 1: Component(), 2: Component() }));
139145

140146
expect(console.warn.argsForCall.length).toBe(1);
141147
expect(console.warn.argsForCall[0][0]).toContain(
@@ -316,4 +322,12 @@ describe('ReactElementValidator', function() {
316322
expect(console.warn.calls.length).toBe(2);
317323
});
318324

325+
it('should warn if a fragment is used without the wrapper', function () {
326+
spyOn(console, 'warn');
327+
var child = React.createElement('span');
328+
React.createElement('div', null, { a: child, b: child });
329+
expect(console.warn.calls.length).toBe(1);
330+
expect(console.warn.calls[0].args[0]).toContain('use of a keyed object');
331+
});
332+
319333
});

‎src/core/__tests__/ReactIdentity-test.js

+9-3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
'use strict';
1313

1414
var React;
15+
var ReactFragment;
1516
var ReactTestUtils;
1617
var reactComponentExpect;
1718
var ReactMount;
@@ -21,6 +22,7 @@ describe('ReactIdentity', function() {
2122
beforeEach(function() {
2223
require('mock-modules').dumpCache();
2324
React = require('React');
25+
ReactFragment = require('ReactFragment');
2426
ReactTestUtils = require('ReactTestUtils');
2527
reactComponentExpect = require('reactComponentExpect');
2628
ReactMount = require('ReactMount');
@@ -35,13 +37,17 @@ describe('ReactIdentity', function() {
3537
expect(actual[1]).toEqual(expected[1]);
3638
}
3739

40+
function frag(obj) {
41+
return ReactFragment.create(obj);
42+
}
43+
3844
it('should allow keyed objects to express identity', function() {
3945
var instance =
4046
<div>
41-
{{
47+
{frag({
4248
first: <div />,
4349
second: <div />
44-
}}
50+
})}
4551
</div>;
4652

4753
instance = React.render(instance, document.createElement('div'));
@@ -106,7 +112,7 @@ describe('ReactIdentity', function() {
106112

107113
var map = {};
108114
map[key] = span2;
109-
return <div>{[span1, map]}</div>;
115+
return <div>{[span1, frag(map)]}</div>;
110116
}
111117

112118
});

‎src/core/__tests__/ReactMultiChildReconcile-test.js

+6-5
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ var stripEmptyValues = function(obj) {
3939
* here. This relies on an implementation detail of the rendering system.
4040
*/
4141
var getOriginalKey = function(childName) {
42-
var match = childName.match(/^\.\$([^.]+)\:0$/);
42+
var match = childName.match(/^\.\$([^.]+)$/);
4343
expect(match).not.toBeNull();
4444
return match[1];
4545
};
@@ -107,13 +107,14 @@ var FriendsStatusDisplay = React.createClass({
107107
return res;
108108
},
109109
render: function() {
110-
var children = null;
110+
var children = [];
111111
var key;
112112
for (key in this.props.usernameToStatus) {
113113
var status = this.props.usernameToStatus[key];
114-
children = children || {};
115-
children[key] = !status ? null :
116-
<StatusDisplay ref={key} status={status} />;
114+
children.push(
115+
!status ? null :
116+
<StatusDisplay key={key} ref={key} status={status} />
117+
);
117118
}
118119
return (
119120
<div>

‎src/core/__tests__/ReactMultiChildText-test.js

+15-12
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,13 @@
1616
require('mock-modules');
1717

1818
var React = require('React');
19+
var ReactFragment = require('ReactFragment');
1920
var ReactTestUtils = require('ReactTestUtils');
2021

2122
var reactComponentExpect = require('reactComponentExpect');
2223

24+
var frag = ReactFragment.create;
25+
2326
// Helpers
2427
var testAllPermutations = function(testCases) {
2528
for (var i = 0; i < testCases.length; i += 2) {
@@ -182,18 +185,18 @@ describe('ReactMultiChildText', function() {
182185
['', 'foo', [true, <div />, 1.2, ''], 'foo'], ['', 'foo', <div />, '1.2', '', 'foo'],
183186

184187
// values inside objects
185-
[{a: true}, {a: true}], [],
186-
[{a: 1.2}, {a: 1.2}], ['1.2', '1.2'],
187-
[{a: ''}, {a: ''}], ['', ''],
188-
[{a: 'foo'}, {a: 'foo'}], ['foo', 'foo'],
189-
[{a: <div />}, {a: <div />}], [<div />, <div />],
190-
191-
[{a: true, b: 1.2, c: <div />}, '', 'foo'], ['1.2', <div />, '', 'foo'],
192-
[1.2, '', {a: <div />, b: 'foo', c: true}], ['1.2', '', <div />, 'foo'],
193-
['', {a: 'foo', b: <div />, c: true}, 1.2], ['', 'foo', <div />, '1.2'],
194-
195-
[true, {a: 1.2, b: '', c: <div />, d: 'foo'}, true, 1.2], ['1.2', '', <div />, 'foo', '1.2'],
196-
['', 'foo', {a: true, b: <div />, c: 1.2, d: ''}, 'foo'], ['', 'foo', <div />, '1.2', '', 'foo'],
188+
[frag({a: true}), frag({a: true})], [],
189+
[frag({a: 1.2}), frag({a: 1.2})], ['1.2', '1.2'],
190+
[frag({a: ''}), frag({a: ''})], ['', ''],
191+
[frag({a: 'foo'}), frag({a: 'foo'})], ['foo', 'foo'],
192+
[frag({a: <div />}), frag({a: <div />})], [<div />, <div />],
193+
194+
[frag({a: true, b: 1.2, c: <div />}), '', 'foo'], ['1.2', <div />, '', 'foo'],
195+
[1.2, '', frag({a: <div />, b: 'foo', c: true})], ['1.2', '', <div />, 'foo'],
196+
['', frag({a: 'foo', b: <div />, c: true}), 1.2], ['', 'foo', <div />, '1.2'],
197+
198+
[true, frag({a: 1.2, b: '', c: <div />, d: 'foo'}), true, 1.2], ['1.2', '', <div />, 'foo', '1.2'],
199+
['', 'foo', frag({a: true, b: <div />, c: 1.2, d: ''}), 'foo'], ['', 'foo', <div />, '1.2', '', 'foo'],
197200

198201
// values inside elements
199202
[<div>{true}{1.2}{<div />}</div>, '', 'foo'], [<div />, '', 'foo'],

‎src/modern/element/__tests__/ReactJSXElementValidator-test.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
// of dynamic errors when using JSX with Flow.
1616

1717
var React;
18+
var ReactFragment;
1819
var ReactTestUtils;
1920

2021
describe('ReactJSXElementValidator', function() {
@@ -24,13 +25,18 @@ describe('ReactJSXElementValidator', function() {
2425
require('mock-modules').dumpCache();
2526

2627
React = require('React');
28+
ReactFragment = require('ReactFragment');
2729
ReactTestUtils = require('ReactTestUtils');
2830

2931
Component = class {
3032
render() { return <div />; }
3133
};
3234
});
3335

36+
function frag(obj) {
37+
return ReactFragment.create(obj);
38+
}
39+
3440
it('warns for keys for arrays of elements in children position', function() {
3541
spyOn(console, 'warn');
3642

@@ -128,7 +134,7 @@ describe('ReactJSXElementValidator', function() {
128134
it('warns for numeric keys on objects as children', function() {
129135
spyOn(console, 'warn');
130136

131-
<Component>{{ 1: <Component />, 2: <Component /> }}</Component>;
137+
<Component>{frag({ 1: <Component />, 2: <Component /> })}</Component>;
132138

133139
expect(console.warn.argsForCall.length).toBe(1);
134140
expect(console.warn.argsForCall[0][0]).toContain(

‎src/utils/ReactChildren.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
'use strict';
1313

1414
var PooledClass = require('PooledClass');
15+
var ReactFragment = require('ReactFragment');
1516

1617
var traverseAllChildren = require('traverseAllChildren');
1718
var warning = require('warning');
@@ -121,7 +122,7 @@ function mapChildren(children, func, context) {
121122
var traverseContext = MapBookKeeping.getPooled(mapResult, func, context);
122123
traverseAllChildren(children, mapSingleChildIntoContext, traverseContext);
123124
MapBookKeeping.release(traverseContext);
124-
return mapResult;
125+
return ReactFragment.create(mapResult);
125126
}
126127

127128
function forEachSingleChildDummy(traverseContext, child, name, i) {

‎src/utils/__tests__/ReactChildren-test.js

+99-34
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,31 @@
1414
describe('ReactChildren', function() {
1515
var ReactChildren;
1616
var React;
17+
var ReactFragment;
1718

1819
beforeEach(function() {
1920
ReactChildren = require('ReactChildren');
2021
React = require('React');
22+
ReactFragment = require('ReactFragment');
2123
});
2224

25+
function frag(obj) {
26+
return ReactFragment.create(obj);
27+
}
28+
29+
function nthChild(mappedChildren, n) {
30+
var result = null;
31+
ReactChildren.forEach(mappedChildren, function(child, index) {
32+
if (index === n) {
33+
result = child;
34+
}
35+
});
36+
return result;
37+
}
38+
39+
function keyOfNthChild(mappedChildren, n) {
40+
return Object.keys(ReactFragment.extract(mappedChildren))[n];
41+
}
2342

2443
it('should support identity for simple', function() {
2544
var callback = jasmine.createSpy().andCallFake(function(kid, index) {
@@ -37,7 +56,7 @@ describe('ReactChildren', function() {
3756
callback.reset();
3857
var mappedChildren = ReactChildren.map(instance.props.children, callback);
3958
expect(callback).toHaveBeenCalledWith(simpleKid, 0);
40-
expect(mappedChildren[Object.keys(mappedChildren)[0]]).toBe(simpleKid);
59+
expect(nthChild(mappedChildren, 0)).toBe(simpleKid);
4160
});
4261

4362
it('should treat single arrayless child as being in array', function() {
@@ -52,7 +71,7 @@ describe('ReactChildren', function() {
5271
callback.reset();
5372
var mappedChildren = ReactChildren.map(instance.props.children, callback);
5473
expect(callback).toHaveBeenCalledWith(simpleKid, 0);
55-
expect(mappedChildren[Object.keys(mappedChildren)[0]]).toBe(simpleKid);
74+
expect(nthChild(mappedChildren, 0)).toBe(simpleKid);
5675
});
5776

5877
it('should treat single child in array as expected', function() {
@@ -67,7 +86,7 @@ describe('ReactChildren', function() {
6786
callback.reset();
6887
var mappedChildren = ReactChildren.map(instance.props.children, callback);
6988
expect(callback).toHaveBeenCalledWith(simpleKid, 0);
70-
expect(mappedChildren[Object.keys(mappedChildren)[0]]).toBe(simpleKid);
89+
expect(nthChild(mappedChildren, 0)).toBe(simpleKid);
7190
});
7291

7392
it('should pass key to returned component', function() {
@@ -80,11 +99,10 @@ describe('ReactChildren', function() {
8099
var instance = <div>{simpleKid}</div>;
81100
var mappedChildren = ReactChildren.map(instance.props.children, mapFn);
82101

83-
var mappedKeys = Object.keys(mappedChildren);
84-
expect(mappedKeys.length).toBe(1);
85-
expect(mappedChildren[mappedKeys[0]]).not.toBe(simpleKid);
86-
expect(mappedChildren[mappedKeys[0]].props.children).toBe(simpleKid);
87-
expect(mappedKeys[0]).toBe('.$simple');
102+
expect(ReactChildren.count(mappedChildren)).toBe(1);
103+
expect(nthChild(mappedChildren, 0)).not.toBe(simpleKid);
104+
expect(nthChild(mappedChildren, 0).props.children).toBe(simpleKid);
105+
expect(keyOfNthChild(mappedChildren, 0)).toBe('.$simple');
88106
});
89107

90108
it('should invoke callback with the right context', function() {
@@ -94,7 +112,8 @@ describe('ReactChildren', function() {
94112
return this;
95113
};
96114

97-
var scopeTester = {};
115+
// TODO: Use an object to test, after non-object fragments has fully landed.
116+
var scopeTester = 'scope tester';
98117

99118
var simpleKid = <span key="simple" />;
100119
var instance = <div>{simpleKid}</div>;
@@ -104,9 +123,8 @@ describe('ReactChildren', function() {
104123
var mappedChildren =
105124
ReactChildren.map(instance.props.children, callback, scopeTester);
106125

107-
var mappedKeys = Object.keys(mappedChildren);
108-
expect(mappedKeys.length).toBe(1);
109-
expect(mappedChildren[mappedKeys[0]]).toBe(scopeTester);
126+
expect(ReactChildren.count(mappedChildren)).toBe(1);
127+
expect(nthChild(mappedChildren, 0)).toBe(scopeTester);
110128
});
111129

112130
it('should be called for each child', function() {
@@ -149,28 +167,33 @@ describe('ReactChildren', function() {
149167

150168
var mappedChildren =
151169
ReactChildren.map(instance.props.children, callback);
152-
var mappedKeys = Object.keys(mappedChildren);
153170
expect(callback.calls.length).toBe(5);
154-
expect(mappedKeys.length).toBe(5);
171+
expect(ReactChildren.count(mappedChildren)).toBe(5);
155172
// Keys default to indices.
156-
expect(mappedKeys).toEqual(
173+
expect([
174+
keyOfNthChild(mappedChildren, 0),
175+
keyOfNthChild(mappedChildren, 1),
176+
keyOfNthChild(mappedChildren, 2),
177+
keyOfNthChild(mappedChildren, 3),
178+
keyOfNthChild(mappedChildren, 4)
179+
]).toEqual(
157180
['.$keyZero', '.1', '.$keyTwo', '.3', '.$keyFour']
158181
);
159182

160183
expect(callback).toHaveBeenCalledWith(zero, 0);
161-
expect(mappedChildren[mappedKeys[0]]).toBe(zeroMapped);
184+
expect(nthChild(mappedChildren, 0)).toBe(zeroMapped);
162185

163186
expect(callback).toHaveBeenCalledWith(one, 1);
164-
expect(mappedChildren[mappedKeys[1]]).toBe(oneMapped);
187+
expect(nthChild(mappedChildren, 1)).toBe(oneMapped);
165188

166189
expect(callback).toHaveBeenCalledWith(two, 2);
167-
expect(mappedChildren[mappedKeys[2]]).toBe(twoMapped);
190+
expect(nthChild(mappedChildren, 2)).toBe(twoMapped);
168191

169192
expect(callback).toHaveBeenCalledWith(three, 3);
170-
expect(mappedChildren[mappedKeys[3]]).toBe(threeMapped);
193+
expect(nthChild(mappedChildren, 3)).toBe(threeMapped);
171194

172195
expect(callback).toHaveBeenCalledWith(four, 4);
173-
expect(mappedChildren[mappedKeys[4]]).toBe(fourMapped);
196+
expect(nthChild(mappedChildren, 4)).toBe(fourMapped);
174197
});
175198

176199

@@ -204,11 +227,11 @@ describe('ReactChildren', function() {
204227

205228
var instance = (
206229
<div>{
207-
[{
230+
[frag({
208231
firstHalfKey: [zero, one, two],
209232
secondHalfKey: [three, four],
210233
keyFive: five
211-
}]
234+
})]
212235
}</div>
213236
);
214237

@@ -222,11 +245,17 @@ describe('ReactChildren', function() {
222245
callback.reset();
223246

224247
var mappedChildren = ReactChildren.map(instance.props.children, callback);
225-
var mappedKeys = Object.keys(mappedChildren);
226248
expect(callback.calls.length).toBe(6);
227-
expect(mappedKeys.length).toBe(6);
249+
expect(ReactChildren.count(mappedChildren)).toBe(6);
228250
// Keys default to indices.
229-
expect(mappedKeys).toEqual([
251+
expect([
252+
keyOfNthChild(mappedChildren, 0),
253+
keyOfNthChild(mappedChildren, 1),
254+
keyOfNthChild(mappedChildren, 2),
255+
keyOfNthChild(mappedChildren, 3),
256+
keyOfNthChild(mappedChildren, 4),
257+
keyOfNthChild(mappedChildren, 5)
258+
]).toEqual([
230259
'.0:$firstHalfKey:0:$keyZero',
231260
'.0:$firstHalfKey:0:1',
232261
'.0:$firstHalfKey:0:$keyTwo',
@@ -236,22 +265,22 @@ describe('ReactChildren', function() {
236265
]);
237266

238267
expect(callback).toHaveBeenCalledWith(zero, 0);
239-
expect(mappedChildren[mappedKeys[0]]).toBe(zeroMapped);
268+
expect(nthChild(mappedChildren, 0)).toBe(zeroMapped);
240269

241270
expect(callback).toHaveBeenCalledWith(one, 1);
242-
expect(mappedChildren[mappedKeys[1]]).toBe(oneMapped);
271+
expect(nthChild(mappedChildren, 1)).toBe(oneMapped);
243272

244273
expect(callback).toHaveBeenCalledWith(two, 2);
245-
expect(mappedChildren[mappedKeys[2]]).toBe(twoMapped);
274+
expect(nthChild(mappedChildren, 2)).toBe(twoMapped);
246275

247276
expect(callback).toHaveBeenCalledWith(three, 3);
248-
expect(mappedChildren[mappedKeys[3]]).toBe(threeMapped);
277+
expect(nthChild(mappedChildren, 3)).toBe(threeMapped);
249278

250279
expect(callback).toHaveBeenCalledWith(four, 4);
251-
expect(mappedChildren[mappedKeys[4]]).toBe(fourMapped);
280+
expect(nthChild(mappedChildren, 4)).toBe(fourMapped);
252281

253282
expect(callback).toHaveBeenCalledWith(five, 5);
254-
expect(mappedChildren[mappedKeys[5]]).toBe(fiveMapped);
283+
expect(nthChild(mappedChildren, 5)).toBe(fiveMapped);
255284
});
256285

257286
it('should retain key across two mappings', function() {
@@ -325,7 +354,8 @@ describe('ReactChildren', function() {
325354
var mapped = ReactChildren.map(instance.props.children, mapFn);
326355

327356
expect(console.warn.calls.length).toEqual(1);
328-
expect(mapped).toEqual({'.$something': zero});
357+
expect(nthChild(mapped, 0)).toBe(zero);
358+
expect(keyOfNthChild(mapped, 0)).toBe('.$something');
329359
});
330360

331361
it('should return 0 for null children', function() {
@@ -380,14 +410,49 @@ describe('ReactChildren', function() {
380410

381411
var instance = (
382412
<div>{
383-
[{
413+
[frag({
384414
firstHalfKey: [zero, one, two],
385415
secondHalfKey: [three, four],
386416
keyFive: five
387-
}]
417+
})]
388418
}</div>
389419
);
390420
var numberOfChildren = ReactChildren.count(instance.props.children);
391421
expect(numberOfChildren).toBe(6);
392422
});
423+
424+
it('should warn if a fragment is used without the wrapper', function () {
425+
spyOn(console, 'warn');
426+
var child = React.createElement('span');
427+
ReactChildren.forEach({ a: child, b: child}, function(c) {
428+
expect(c).toBe(child);
429+
});
430+
expect(console.warn.calls.length).toBe(1);
431+
expect(console.warn.calls[0].args[0]).toContain('use of a keyed object');
432+
});
433+
434+
it('should warn if a fragment is accessed', function () {
435+
spyOn(console, 'warn');
436+
var child = React.createElement('span');
437+
var frag = ReactChildren.map([ child, child ], function(c) {
438+
return c;
439+
});
440+
for (var key in frag) {
441+
frag[key];
442+
break;
443+
}
444+
expect(console.warn.calls.length).toBe(1);
445+
expect(console.warn.calls[0].args[0]).toContain('is an opaque type');
446+
447+
var frag2 = ReactChildren.map([ child, child ], function(c) {
448+
return c;
449+
});
450+
for (var key in frag2) {
451+
frag2[key] = 123;
452+
break;
453+
}
454+
expect(console.warn.calls.length).toBe(2);
455+
expect(console.warn.calls[1].args[0]).toContain('is an immutable opaque');
456+
});
457+
393458
});

‎src/utils/__tests__/onlyChild-test.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@
1414
describe('onlyChild', function() {
1515

1616
var React;
17+
var ReactFragment;
1718
var onlyChild;
1819
var WrapComponent;
1920

2021
beforeEach(function() {
2122
React = require('React');
23+
ReactFragment = require('ReactFragment');
2224
onlyChild = require('onlyChild');
2325
WrapComponent = React.createClass({
2426
render: function() {
@@ -64,7 +66,7 @@ describe('onlyChild', function() {
6466
expect(function() {
6567
var instance =
6668
<WrapComponent>
67-
{{oneThing: <span />}}
69+
{ReactFragment.create({oneThing: <span />})}
6870
</WrapComponent>;
6971
onlyChild(instance.props.children);
7072
}).toThrow();

‎src/utils/__tests__/sliceChildren-test.js

+11-4
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
describe('sliceChildren', function() {
1515

1616
var React;
17+
var ReactFragment;
1718
var ReactTestUtils;
1819

1920
var sliceChildren;
@@ -23,6 +24,7 @@ describe('sliceChildren', function() {
2324

2425
beforeEach(function() {
2526
React = require('React');
27+
ReactFragment = require('ReactFragment');
2628
ReactTestUtils = require('ReactTestUtils');
2729

2830
sliceChildren = require('sliceChildren');
@@ -52,14 +54,19 @@ describe('sliceChildren', function() {
5254
return rendered.props.children;
5355
}
5456

57+
function testKeyValuePairs(children, expectedPairs) {
58+
var obj = ReactFragment.extract(children);
59+
expect(obj).toEqual(expectedPairs);
60+
}
61+
5562
it('should render the whole set if start zero is supplied', function() {
5663
var fullSet = [
5764
<div key="A" />,
5865
<div key="B" />,
5966
<div key="C" />
6067
];
6168
var children = renderAndSlice(fullSet, 0);
62-
expect(children).toEqual({
69+
testKeyValuePairs(children, {
6370
'.$A': fullSet[0],
6471
'.$B': fullSet[1],
6572
'.$C': fullSet[2]
@@ -73,7 +80,7 @@ describe('sliceChildren', function() {
7380
<div key="C" />
7481
];
7582
var children = renderAndSlice(fullSet, 1);
76-
expect(children).toEqual({
83+
testKeyValuePairs(children, {
7784
'.$B': fullSet[1],
7885
'.$C': fullSet[2]
7986
});
@@ -87,7 +94,7 @@ describe('sliceChildren', function() {
8794
<div key="D" />
8895
];
8996
var children = renderAndSlice(fullSet, 1, 2);
90-
expect(children).toEqual({
97+
testKeyValuePairs(children, {
9198
'.$B': fullSet[1]
9299
});
93100
});
@@ -103,7 +110,7 @@ describe('sliceChildren', function() {
103110
.expectRenderedChild()
104111
.instance();
105112

106-
expect(rendered.props.children).toEqual({
113+
testKeyValuePairs(rendered.props.children, {
107114
'.1': b
108115
});
109116
});

‎src/utils/__tests__/traverseAllChildren-test.js

+9-4
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,16 @@
1414
describe('traverseAllChildren', function() {
1515
var traverseAllChildren;
1616
var React;
17+
var ReactFragment;
1718
beforeEach(function() {
1819
traverseAllChildren = require('traverseAllChildren');
1920
React = require('React');
21+
ReactFragment = require('ReactFragment');
2022
});
2123

24+
function frag(obj) {
25+
return ReactFragment.create(obj);
26+
}
2227

2328
it('should support identity for simple', function() {
2429
var traverseContext = [];
@@ -139,8 +144,8 @@ describe('traverseAllChildren', function() {
139144
var instance = (
140145
<div>
141146
{div}
142-
{[{span}]}
143-
{{a: a}}
147+
{[frag({span})]}
148+
{frag({a: a})}
144149
{'string'}
145150
{1234}
146151
{true}
@@ -206,11 +211,11 @@ describe('traverseAllChildren', function() {
206211

207212
var instance = (
208213
<div>{
209-
[{
214+
[frag({
210215
firstHalfKey: [zero, one, two],
211216
secondHalfKey: [three, four],
212217
keyFive: five
213-
}]
218+
})]
214219
}</div>
215220
);
216221

‎src/utils/sliceChildren.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
'use strict';
1313

14+
var ReactFragment = require('ReactFragment');
15+
1416
var flattenChildren = require('flattenChildren');
1517

1618
/**
@@ -43,7 +45,7 @@ function sliceChildren(children, start, end) {
4345
break;
4446
}
4547
}
46-
return slicedChildren;
48+
return ReactFragment.create(slicedChildren);
4749
}
4850

4951
module.exports = sliceChildren;

‎src/utils/traverseAllChildren.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
'use strict';
1313

1414
var ReactElement = require('ReactElement');
15+
var ReactFragment = require('ReactFragment');
1516
var ReactInstanceHandles = require('ReactInstanceHandles');
1617

1718
var getIteratorFn = require('getIteratorFn');
@@ -184,9 +185,10 @@ function traverseAllChildrenImpl(
184185
'traverseAllChildren(...): Encountered an invalid child; DOM ' +
185186
'elements are not valid children of React components.'
186187
);
187-
for (var key in children) {
188-
if (children.hasOwnProperty(key)) {
189-
child = children[key];
188+
var fragment = ReactFragment.extract(children);
189+
for (var key in fragment) {
190+
if (fragment.hasOwnProperty(key)) {
191+
child = fragment[key];
190192
nextName = (
191193
(nameSoFar !== '' ? nameSoFar + SUBSEPARATOR : SEPARATOR) +
192194
wrapUserProvidedKey(key) + SUBSEPARATOR +

0 commit comments

Comments
 (0)
Please sign in to comment.