Skip to content

Commit c07ea0b

Browse files
leebyronleebyron
committed
Allow iterables in traverseAllChildren
This lets you use any kind of iterable as a container of react child elements. It also preserves keys when possible if the iterable is keyed. * Use an ES6 Map or Set to hold children. * Use an Immutable-js collection to hold children. * Use a function* which yields children. Concretely, this removes the necessary conversion to JS objects if you're mapping over Immutable-js collections to get children, i.e.: ``` <div> {myImmutable.map(val => <span>{val}</span>).toJS() </div> ``` Now we can remove the `toJS()` and the intermediate allocation along with it. This also cleans up the traverseAllChildrenImpl method.
1 parent daf4182 commit c07ea0b

5 files changed

Lines changed: 427 additions & 57 deletions

File tree

‎src/core/ReactElementValidator.js‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ var ReactElement = require('ReactElement');
2222
var ReactPropTypeLocations = require('ReactPropTypeLocations');
2323
var ReactCurrentOwner = require('ReactCurrentOwner');
2424

25+
var getIteratorFn = require('getIteratorFn');
2526
var monitorCodeUse = require('monitorCodeUse');
2627

2728
/**
@@ -68,7 +69,7 @@ function validateExplicitKey(component, parentType) {
6869

6970
warnAndMonitorForKeyUse(
7071
'react_key_warning',
71-
'Each child in an array should have a unique "key" prop.',
72+
'Each child in an array or iterator should have a unique "key" prop.',
7273
component,
7374
parentType
7475
);
@@ -123,7 +124,9 @@ function warnAndMonitorForKeyUse(warningID, message, component, parentType) {
123124
// property, it may be the creator of the child that's responsible for
124125
// assigning it a key.
125126
var childOwnerName = null;
126-
if (component._owner && component._owner !== ReactCurrentOwner.current) {
127+
if (component &&
128+
component._owner &&
129+
component._owner !== ReactCurrentOwner.current) {
127130
// Name of the component that originally created this child.
128131
childOwnerName = component._owner.constructor.displayName;
129132

@@ -161,7 +164,6 @@ function monitorUseOfObjectMap() {
161164
* @internal
162165
* @param {*} component Statically passed child of any type.
163166
* @param {*} parentType component's parent's type.
164-
* @return {boolean}
165167
*/
166168
function validateChildKeys(component, parentType) {
167169
if (Array.isArray(component)) {
@@ -174,10 +176,24 @@ function validateChildKeys(component, parentType) {
174176
} else if (ReactElement.isValidElement(component)) {
175177
// This component was passed in a valid location.
176178
component._store.validated = true;
177-
} else if (component && typeof component === 'object') {
178-
monitorUseOfObjectMap();
179-
for (var name in component) {
180-
validatePropertyKey(name, component[name], parentType);
179+
} else if (component) {
180+
var iteratorFn = getIteratorFn(component);
181+
// Entry iterators provide implicit keys.
182+
if (iteratorFn && iteratorFn !== component.entries) {
183+
var iterator = iteratorFn.call(component);
184+
var step;
185+
while (!(step = iterator.next()).done) {
186+
if (ReactElement.isValidElement(step.value)) {
187+
validateExplicitKey(step.value, parentType);
188+
}
189+
}
190+
} else if (typeof component === 'object') {
191+
monitorUseOfObjectMap();
192+
for (var key in component) {
193+
if (component.hasOwnProperty(key)) {
194+
validatePropertyKey(key, component[key], parentType);
195+
}
196+
}
181197
}
182198
}
183199
}

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

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,101 @@ describe('ReactElement', function() {
169169

170170
expect(console.warn.argsForCall.length).toBe(1);
171171
expect(console.warn.argsForCall[0][0]).toContain(
172-
'Each child in an array should have a unique "key" prop'
172+
'Each child in an array or iterator should have a unique "key" prop.'
173173
);
174174
});
175175

176+
it('warns for keys for iterables of elements in rest args', function() {
177+
spyOn(console, 'warn');
178+
var Component = React.createFactory(ComponentFactory);
179+
180+
var iterable = {
181+
'@@iterator': function() {
182+
var i = 0;
183+
return {
184+
next: function() {
185+
var done = ++i > 2;
186+
return { value: done ? undefined : Component(), done: done };
187+
}
188+
};
189+
}
190+
};
191+
192+
Component(null, iterable);
193+
194+
expect(console.warn.argsForCall.length).toBe(1);
195+
expect(console.warn.argsForCall[0][0]).toContain(
196+
'Each child in an array or iterator should have a unique "key" prop.'
197+
);
198+
});
199+
200+
it('does not warns for arrays of elements with keys', function() {
201+
spyOn(console, 'warn');
202+
var Component = React.createFactory(ComponentFactory);
203+
204+
Component(null, [ Component({key: '#1'}), Component({key: '#2'}) ]);
205+
206+
expect(console.warn.argsForCall.length).toBe(0);
207+
});
208+
209+
it('does not warns for iterable elements with keys', function() {
210+
spyOn(console, 'warn');
211+
var Component = React.createFactory(ComponentFactory);
212+
213+
var iterable = {
214+
'@@iterator': function() {
215+
var i = 0;
216+
return {
217+
next: function() {
218+
var done = ++i > 2;
219+
return {
220+
value: done ? undefined : Component({key: '#' + i}),
221+
done: done
222+
};
223+
}
224+
};
225+
}
226+
};
227+
228+
Component(null, iterable);
229+
230+
expect(console.warn.argsForCall.length).toBe(0);
231+
});
232+
233+
it('warns for numeric keys on objects in rest args', function() {
234+
spyOn(console, 'warn');
235+
var Component = React.createFactory(ComponentFactory);
236+
237+
Component(null, { 1: Component(), 2: Component() });
238+
239+
expect(console.warn.argsForCall.length).toBe(1);
240+
expect(console.warn.argsForCall[0][0]).toContain(
241+
'Child objects should have non-numeric keys so ordering is preserved.'
242+
);
243+
});
244+
245+
it('does not warn for numeric keys in entry iterables in rest args', function() {
246+
spyOn(console, 'warn');
247+
var Component = React.createFactory(ComponentFactory);
248+
249+
var iterable = {
250+
'@@iterator': function() {
251+
var i = 0;
252+
return {
253+
next: function() {
254+
var done = ++i > 2;
255+
return { value: done ? undefined : [i, Component()], done: done };
256+
}
257+
};
258+
}
259+
};
260+
iterable.entries = iterable['@@iterator'];
261+
262+
Component(null, iterable);
263+
264+
expect(console.warn.argsForCall.length).toBe(0);
265+
});
266+
176267
it('does not warn when the element is directly in rest args', function() {
177268
spyOn(console, 'warn');
178269
var Component = React.createFactory(ComponentFactory);

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

Lines changed: 161 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,64 @@ describe('traverseAllChildren', function() {
125125
);
126126
});
127127

128-
// Todo: test that nums/strings are converted to ReactComponents.
128+
it('should traverse children of different kinds', function() {
129+
var div = <div key="divNode" />;
130+
var span = <span key="spanNode" />;
131+
var a = <a key="aNode" />;
132+
133+
var traverseContext = [];
134+
var traverseFn =
135+
jasmine.createSpy().andCallFake(function(context, kid, key, index) {
136+
context.push(true);
137+
});
138+
139+
var instance = (
140+
<div>
141+
{div}
142+
{[{span}]}
143+
{{a: a}}
144+
{'string'}
145+
{1234}
146+
{true}
147+
{false}
148+
{null}
149+
{undefined}
150+
</div>
151+
);
152+
153+
traverseAllChildren(instance.props.children, traverseFn, traverseContext);
154+
155+
expect(traverseFn.calls.length).toBe(9);
156+
expect(traverseContext.length).toEqual(9);
157+
158+
expect(traverseFn).toHaveBeenCalledWith(
159+
traverseContext, div, '.$divNode', 0
160+
);
161+
expect(traverseFn).toHaveBeenCalledWith(
162+
traverseContext, span, '.1:0:$span:$spanNode', 1
163+
);
164+
expect(traverseFn).toHaveBeenCalledWith(
165+
traverseContext, a, '.2:$a:$aNode', 2
166+
);
167+
expect(traverseFn).toHaveBeenCalledWith(
168+
traverseContext, 'string', '.3', 3
169+
);
170+
expect(traverseFn).toHaveBeenCalledWith(
171+
traverseContext, 1234, '.4', 4
172+
);
173+
expect(traverseFn).toHaveBeenCalledWith(
174+
traverseContext, null, '.5', 5
175+
);
176+
expect(traverseFn).toHaveBeenCalledWith(
177+
traverseContext, null, '.6', 6
178+
);
179+
expect(traverseFn).toHaveBeenCalledWith(
180+
traverseContext, null, '.7', 7
181+
);
182+
expect(traverseFn).toHaveBeenCalledWith(
183+
traverseContext, null, '.8', 8
184+
);
185+
});
129186

130187
it('should be called for each child in nested structure', function() {
131188
var zero = <div key="keyZero" />;
@@ -231,4 +288,107 @@ describe('traverseAllChildren', function() {
231288
);
232289
});
233290

291+
it('should be called for each child in an iterable', function() {
292+
var threeDivIterable = {
293+
'@@iterator': function() {
294+
var i = 0;
295+
return {
296+
next: function() {
297+
if (i++ < 3) {
298+
return { value: <div key={'#'+i} />, done: false };
299+
} else {
300+
return { value: undefined, done: true };
301+
}
302+
}
303+
};
304+
}
305+
};
306+
307+
var traverseContext = [];
308+
var traverseFn =
309+
jasmine.createSpy().andCallFake(function(context, kid, key, index) {
310+
context.push(kid);
311+
});
312+
313+
var instance = (
314+
<div>
315+
{threeDivIterable}
316+
</div>
317+
);
318+
319+
traverseAllChildren(instance.props.children, traverseFn, traverseContext);
320+
expect(traverseFn.calls.length).toBe(3);
321+
322+
expect(traverseFn).toHaveBeenCalledWith(
323+
traverseContext,
324+
traverseContext[0],
325+
'.$#1',
326+
0
327+
);
328+
expect(traverseFn).toHaveBeenCalledWith(
329+
traverseContext,
330+
traverseContext[1],
331+
'.$#2',
332+
1
333+
);
334+
expect(traverseFn).toHaveBeenCalledWith(
335+
traverseContext,
336+
traverseContext[2],
337+
'.$#3',
338+
2
339+
);
340+
});
341+
342+
it('should use keys from entry iterables', function() {
343+
var threeDivEntryIterable = {
344+
'@@iterator': function() {
345+
var i = 0;
346+
return {
347+
next: function() {
348+
if (i++ < 3) {
349+
return { value: ['#'+i, <div />], done: false };
350+
} else {
351+
return { value: undefined, done: true };
352+
}
353+
}
354+
};
355+
}
356+
};
357+
threeDivEntryIterable.entries = threeDivEntryIterable['@@iterator'];
358+
359+
var traverseContext = [];
360+
var traverseFn =
361+
jasmine.createSpy().andCallFake(function(context, kid, key, index) {
362+
context.push(kid);
363+
});
364+
365+
var instance = (
366+
<div>
367+
{threeDivEntryIterable}
368+
</div>
369+
);
370+
371+
traverseAllChildren(instance.props.children, traverseFn, traverseContext);
372+
expect(traverseFn.calls.length).toBe(3);
373+
374+
expect(traverseFn).toHaveBeenCalledWith(
375+
traverseContext,
376+
traverseContext[0],
377+
'.$#1:0',
378+
0
379+
);
380+
expect(traverseFn).toHaveBeenCalledWith(
381+
traverseContext,
382+
traverseContext[1],
383+
'.$#2:0',
384+
1
385+
);
386+
expect(traverseFn).toHaveBeenCalledWith(
387+
traverseContext,
388+
traverseContext[2],
389+
'.$#3:0',
390+
2
391+
);
392+
});
393+
234394
});

‎src/utils/getIteratorFn.js‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/**
2+
* Copyright 2013-2014, 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 getIteratorFn
10+
* @typechecks static-only
11+
*/
12+
13+
"use strict";
14+
15+
/* global Symbol */
16+
var ITERATOR_SYMBOL = typeof Symbol === 'function' && Symbol.iterator;
17+
var FAUX_ITERATOR_SYMBOL = '@@iterator'; // Before Symbol spec.
18+
19+
/**
20+
* Returns the iterator method function contained on the iterable object.
21+
*
22+
* Be sure to invoke the function with the iterable as context:
23+
*
24+
* var iteratorFn = getIteratorFn(myIterable);
25+
* if (iteratorFn) {
26+
* var iterator = iteratorFn.call(myIterable);
27+
* ...
28+
* }
29+
*
30+
* @param {?object} maybeIterable
31+
* @return {?function}
32+
*/
33+
function getIteratorFn(maybeIterable) {
34+
var iteratorFn = maybeIterable && (
35+
(ITERATOR_SYMBOL && maybeIterable[ITERATOR_SYMBOL]) ||
36+
maybeIterable[FAUX_ITERATOR_SYMBOL]
37+
);
38+
if (typeof iteratorFn === 'function') {
39+
return iteratorFn;
40+
}
41+
}
42+
43+
module.exports = getIteratorFn;

0 commit comments

Comments
 (0)