@samitouri / QOS-React-2 / commits / ce215483ae

Don't thread index through traverseAllChildren

I'm not super attached to this, but this feels cleaner to me. Might even be faster since flattenChildren doesn't use the index. Probably no change though.

Ben Alpert committed Apr 11, 2015 at 01:04 UTC ce215483aec5a060f746f7ec5f6071ab616e2a5f
3 files changed +53 -84
src/utils/ReactChildren.js
+15 -14
@@ -29,15 +29,15 @@ var threeArgumentPooler = PooledClass.threeArgumentPooler;
29 * @param {?*} forEachContext Context to perform context with.
30 */
31 function ForEachBookKeeping(forEachFunction, forEachContext) {
32 - this.forEachFunction = forEachFunction;
33 - this.forEachContext = forEachContext;
32 + this.func = forEachFunction;
33 + this.context = forEachContext;
34 + this.count = 0;
35 }
36 PooledClass.addPoolingTo(ForEachBookKeeping, twoArgumentPooler);
37
37 -function forEachSingleChild(traverseContext, child, name, i) {
38 - var forEachBookKeeping = traverseContext;
39 - forEachBookKeeping.forEachFunction.call(
40 - forEachBookKeeping.forEachContext, child, i);
38 +function forEachSingleChild(traverseContext, child, name) {
39 + var bookKeeping = traverseContext;
40 + bookKeeping.func.call(bookKeeping.context, child, bookKeeping.count++);
41 }
42
43 /**
@@ -71,15 +71,16 @@ function forEachChildren(children, forEachFunc, forEachContext) {
71 * @param {?*} mapContext Context to perform mapping with.
72 */
73 function MapBookKeeping(mapResult, mapFunction, mapContext) {
74 - this.mapResult = mapResult;
75 - this.mapFunction = mapFunction;
76 - this.mapContext = mapContext;
74 + this.result = mapResult;
75 + this.func = mapFunction;
76 + this.context = mapContext;
77 + this.count = 0;
78 }
79 PooledClass.addPoolingTo(MapBookKeeping, threeArgumentPooler);
80
80 -function mapSingleChildIntoContext(traverseContext, child, name, i) {
81 - var mapBookKeeping = traverseContext;
82 - var mapResult = mapBookKeeping.mapResult;
81 +function mapSingleChildIntoContext(traverseContext, child, name) {
82 + var bookKeeping = traverseContext;
83 + var mapResult = bookKeeping.result;
84
85 var keyUnique = !mapResult.hasOwnProperty(name);
86 if (__DEV__) {
@@ -94,7 +95,7 @@ function mapSingleChildIntoContext(traverseContext, child, name, i) {
95
96 if (keyUnique) {
97 var mappedChild =
97 - mapBookKeeping.mapFunction.call(mapBookKeeping.mapContext, child, i);
98 + bookKeeping.func.call(bookKeeping.context, child, bookKeeping.count++);
99 mapResult[name] = mappedChild;
100 }
101 }
@@ -125,7 +126,7 @@ function mapChildren(children, func, context) {
126 return ReactFragment.create(mapResult);
127 }
128
128 -function forEachSingleChildDummy(traverseContext, child, name, i) {
129 +function forEachSingleChildDummy(traverseContext, child, name) {
130 return null;
131 }
132
src/utils/__tests__/traverseAllChildren-test.js
+34 -56
@@ -40,8 +40,7 @@ describe('traverseAllChildren', function() {
40 expect(traverseFn).toHaveBeenCalledWith(
41 traverseContext,
42 simpleKid,
43 - '.$simple',
44 - 0
43 + '.$simple'
44 );
45 expect(traverseContext.length).toEqual(1);
46 });
@@ -59,8 +58,7 @@ describe('traverseAllChildren', function() {
58 expect(traverseFn).toHaveBeenCalledWith(
59 traverseContext,
60 simpleKid,
62 - '.0',
63 - 0
61 + '.0'
62 );
63 expect(traverseContext.length).toEqual(1);
64 });
@@ -78,8 +76,7 @@ describe('traverseAllChildren', function() {
76 expect(traverseFn).toHaveBeenCalledWith(
77 traverseContext,
78 simpleKid,
81 - '.0',
82 - 0
79 + '.0'
80 );
81 expect(traverseContext.length).toEqual(1);
82 });
@@ -111,22 +108,19 @@ describe('traverseAllChildren', function() {
108 expect(traverseFn).toHaveBeenCalledWith(
109 traverseContext,
110 zero,
114 - '.$keyZero',
115 - 0
111 + '.$keyZero'
112 );
117 - expect(traverseFn).toHaveBeenCalledWith(traverseContext, one, '.1', 1);
113 + expect(traverseFn).toHaveBeenCalledWith(traverseContext, one, '.1');
114 expect(traverseFn).toHaveBeenCalledWith(
115 traverseContext,
116 two,
121 - '.$keyTwo',
122 - 2
117 + '.$keyTwo'
118 );
124 - expect(traverseFn).toHaveBeenCalledWith(traverseContext, three, '.3', 3);
119 + expect(traverseFn).toHaveBeenCalledWith(traverseContext, three, '.3');
120 expect(traverseFn).toHaveBeenCalledWith(
121 traverseContext,
122 four,
128 - '.$keyFour',
129 - 4
123 + '.$keyFour'
124 );
125 });
126
@@ -161,31 +155,31 @@ describe('traverseAllChildren', function() {
155 expect(traverseContext.length).toEqual(9);
156
157 expect(traverseFn).toHaveBeenCalledWith(
164 - traverseContext, div, '.$divNode', 0
158 + traverseContext, div, '.$divNode'
159 );
160 expect(traverseFn).toHaveBeenCalledWith(
167 - traverseContext, span, '.1:0:$span:$spanNode', 1
161 + traverseContext, span, '.1:0:$span:$spanNode'
162 );
163 expect(traverseFn).toHaveBeenCalledWith(
170 - traverseContext, a, '.2:$a:$aNode', 2
164 + traverseContext, a, '.2:$a:$aNode'
165 );
166 expect(traverseFn).toHaveBeenCalledWith(
173 - traverseContext, 'string', '.3', 3
167 + traverseContext, 'string', '.3'
168 );
169 expect(traverseFn).toHaveBeenCalledWith(
176 - traverseContext, 1234, '.4', 4
170 + traverseContext, 1234, '.4'
171 );
172 expect(traverseFn).toHaveBeenCalledWith(
179 - traverseContext, null, '.5', 5
173 + traverseContext, null, '.5'
174 );
175 expect(traverseFn).toHaveBeenCalledWith(
182 - traverseContext, null, '.6', 6
176 + traverseContext, null, '.6'
177 );
178 expect(traverseFn).toHaveBeenCalledWith(
185 - traverseContext, null, '.7', 7
179 + traverseContext, null, '.7'
180 );
181 expect(traverseFn).toHaveBeenCalledWith(
188 - traverseContext, null, '.8', 8
182 + traverseContext, null, '.8'
183 );
184 });
185
@@ -225,39 +219,34 @@ describe('traverseAllChildren', function() {
219 expect(traverseFn).toHaveBeenCalledWith(
220 traverseContext,
221 zero,
228 - '.0:$firstHalfKey:0:$keyZero',
229 - 0
222 + '.0:$firstHalfKey:0:$keyZero'
223 );
224
225 expect(traverseFn)
233 - .toHaveBeenCalledWith(traverseContext, one, '.0:$firstHalfKey:0:1', 1);
226 + .toHaveBeenCalledWith(traverseContext, one, '.0:$firstHalfKey:0:1');
227
228 expect(traverseFn).toHaveBeenCalledWith(
229 traverseContext,
230 two,
238 - '.0:$firstHalfKey:0:$keyTwo',
239 - 2
231 + '.0:$firstHalfKey:0:$keyTwo'
232 );
233
234 expect(traverseFn).toHaveBeenCalledWith(
235 traverseContext,
236 three,
245 - '.0:$secondHalfKey:0:0',
246 - 3
237 + '.0:$secondHalfKey:0:0'
238 );
239
240 expect(traverseFn).toHaveBeenCalledWith(
241 traverseContext,
242 four,
252 - '.0:$secondHalfKey:0:$keyFour',
253 - 4
243 + '.0:$secondHalfKey:0:$keyFour'
244 );
245
246 expect(traverseFn).toHaveBeenCalledWith(
247 traverseContext,
248 five,
259 - '.0:$keyFive:$keyFiveInner',
260 - 5
249 + '.0:$keyFive:$keyFiveInner'
250 );
251 });
252
@@ -282,14 +271,12 @@ describe('traverseAllChildren', function() {
271 expect(traverseFn).toHaveBeenCalledWith(
272 traverseContext,
273 zeroForceKey,
285 - '.$keyZero',
286 - 0
274 + '.$keyZero'
275 );
276 expect(traverseFn).toHaveBeenCalledWith(
277 traverseContext,
278 oneForceKey,
291 - '.$keyOne',
292 - 1
279 + '.$keyOne'
280 );
281 });
282
@@ -327,20 +314,17 @@ describe('traverseAllChildren', function() {
314 expect(traverseFn).toHaveBeenCalledWith(
315 traverseContext,
316 traverseContext[0],
330 - '.0',
331 - 0
317 + '.0'
318 );
319 expect(traverseFn).toHaveBeenCalledWith(
320 traverseContext,
321 traverseContext[1],
336 - '.1',
337 - 1
322 + '.1'
323 );
324 expect(traverseFn).toHaveBeenCalledWith(
325 traverseContext,
326 traverseContext[2],
342 - '.2',
343 - 2
327 + '.2'
328 );
329 });
330
@@ -378,20 +362,17 @@ describe('traverseAllChildren', function() {
362 expect(traverseFn).toHaveBeenCalledWith(
363 traverseContext,
364 traverseContext[0],
381 - '.$#1',
382 - 0
365 + '.$#1'
366 );
367 expect(traverseFn).toHaveBeenCalledWith(
368 traverseContext,
369 traverseContext[1],
387 - '.$#2',
388 - 1
370 + '.$#2'
371 );
372 expect(traverseFn).toHaveBeenCalledWith(
373 traverseContext,
374 traverseContext[2],
393 - '.$#3',
394 - 2
375 + '.$#3'
376 );
377 });
378
@@ -432,20 +413,17 @@ describe('traverseAllChildren', function() {
413 expect(traverseFn).toHaveBeenCalledWith(
414 traverseContext,
415 traverseContext[0],
435 - '.$#1:0',
436 - 0
416 + '.$#1:0'
417 );
418 expect(traverseFn).toHaveBeenCalledWith(
419 traverseContext,
420 traverseContext[1],
441 - '.$#2:0',
442 - 1
421 + '.$#2:0'
422 );
423 expect(traverseFn).toHaveBeenCalledWith(
424 traverseContext,
425 traverseContext[2],
447 - '.$#3:0',
448 - 2
426 + '.$#3:0'
427 );
428
429 expect(console.error.argsForCall.length).toBe(1);
src/utils/traverseAllChildren.js
+4 -14
@@ -84,7 +84,6 @@ function wrapUserProvidedKey(key) {
84 /**
85 * @param {?*} children Children tree container.
86 * @param {!string} nameSoFar Name of the key path so far.
87 - * @param {!number} indexSoFar Number of children encountered until this point.
87 * @param {!function} callback Callback to invoke with each child found.
88 * @param {?*} traverseContext Used to pass information throughout the traversal
89 * process.
@@ -93,7 +92,6 @@ function wrapUserProvidedKey(key) {
92 function traverseAllChildrenImpl(
93 children,
94 nameSoFar,
96 - indexSoFar,
95 callback,
96 traverseContext
97 ) {
@@ -113,13 +111,13 @@ function traverseAllChildrenImpl(
111 children,
112 // If it's the only child, treat the name as if it was wrapped in an array
113 // so that it's consistent if the number of children grows.
116 - nameSoFar === '' ? SEPARATOR + getComponentKey(children, 0) : nameSoFar,
117 - indexSoFar
114 + nameSoFar === '' ? SEPARATOR + getComponentKey(children, 0) : nameSoFar
115 );
116 return 1;
117 }
118
122 - var child, nextName, nextIndex;
119 + var child;
120 + var nextName;
121 var subtreeCount = 0; // Count of children found in the current subtree.
122
123 if (Array.isArray(children)) {
@@ -129,11 +127,9 @@ function traverseAllChildrenImpl(
127 (nameSoFar !== '' ? nameSoFar + SUBSEPARATOR : SEPARATOR) +
128 getComponentKey(child, i)
129 );
132 - nextIndex = indexSoFar + subtreeCount;
130 subtreeCount += traverseAllChildrenImpl(
131 child,
132 nextName,
136 - nextIndex,
133 callback,
134 traverseContext
135 );
@@ -151,11 +147,9 @@ function traverseAllChildrenImpl(
147 (nameSoFar !== '' ? nameSoFar + SUBSEPARATOR : SEPARATOR) +
148 getComponentKey(child, ii++)
149 );
154 - nextIndex = indexSoFar + subtreeCount;
150 subtreeCount += traverseAllChildrenImpl(
151 child,
152 nextName,
158 - nextIndex,
153 callback,
154 traverseContext
155 );
@@ -180,11 +174,9 @@ function traverseAllChildrenImpl(
174 wrapUserProvidedKey(entry[0]) + SUBSEPARATOR +
175 getComponentKey(child, 0)
176 );
183 - nextIndex = indexSoFar + subtreeCount;
177 subtreeCount += traverseAllChildrenImpl(
178 child,
179 nextName,
187 - nextIndex,
180 callback,
181 traverseContext
182 );
@@ -206,11 +198,9 @@ function traverseAllChildrenImpl(
198 wrapUserProvidedKey(key) + SUBSEPARATOR +
199 getComponentKey(child, 0)
200 );
209 - nextIndex = indexSoFar + subtreeCount;
201 subtreeCount += traverseAllChildrenImpl(
202 child,
203 nextName,
213 - nextIndex,
204 callback,
205 traverseContext
206 );
@@ -243,7 +233,7 @@ function traverseAllChildren(children, callback, traverseContext) {
233 return 0;
234 }
235
246 - return traverseAllChildrenImpl(children, '', 0, callback, traverseContext);
236 + return traverseAllChildrenImpl(children, '', callback, traverseContext);
237 }
238
239 module.exports = traverseAllChildren;