[traverseAllChildren] fix out-of-scope var use.
Dear ES6 gods, bring us `let` soon. This fixes an issue where non-keyed iterables are used as children and the value of `i` would be undefined because its used out of scope. This adds a separately scoped iteration index value and appropriately increments it as iteration continues. Doi. While I'm in there, make the usage of falsey `nameSoFar` more obvious and more consistent with the existing usage on L115 Test plan: first wrote a test covering this previously untested path. Saw an identical issue as was experienced in development environment. Then ensured test passed after this diff.
Lee Byron committed
Nov 3, 2014 at 21:16 UTC
a3608d27ab657ab15809c969a97c3977a0f85b80
2 files changed
+58
-8
src/utils/__tests__/traverseAllChildren-test.js
+52
-1
@@ -288,7 +288,58 @@ describe('traverseAllChildren', function() {
288
);
289
});
290
291
- it('should be called for each child in an iterable', function() {
291
+ it('should be called for each child in an iterable without keys', function() {
292
+ var threeDivIterable = {
293
+ '@@iterator': function() {
294
+ var i = 0;
295
+ return {
296
+ next: function() {
297
+ if (i++ < 3) {
298
+ return { value: <div />, 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
+ '.0',
326
+ 0
327
+ );
328
+ expect(traverseFn).toHaveBeenCalledWith(
329
+ traverseContext,
330
+ traverseContext[1],
331
+ '.1',
332
+ 1
333
+ );
334
+ expect(traverseFn).toHaveBeenCalledWith(
335
+ traverseContext,
336
+ traverseContext[2],
337
+ '.2',
338
+ 2
339
+ );
340
+ });
341
+
342
+ it('should be called for each child in an iterable with keys', function() {
343
var threeDivIterable = {
344
'@@iterator': function() {
345
var i = 0;
src/utils/traverseAllChildren.js
+6
-7
@@ -125,8 +125,7 @@ function traverseAllChildrenImpl(
125
for (var i = 0; i < children.length; i++) {
126
child = children[i];
127
nextName = (
128
- nameSoFar +
129
- (nameSoFar ? SUBSEPARATOR : SEPARATOR) +
128
+ (nameSoFar !== '' ? nameSoFar + SUBSEPARATOR : SEPARATOR) +
129
getComponentKey(child, i)
130
);
131
nextIndex = indexSoFar + subtreeCount;
@@ -144,12 +143,12 @@ function traverseAllChildrenImpl(
143
var iterator = iteratorFn.call(children);
144
var step;
145
if (iteratorFn !== children.entries) {
146
+ var ii = 0;
147
while (!(step = iterator.next()).done) {
148
child = step.value;
149
nextName = (
150
- nameSoFar +
151
- (nameSoFar ? SUBSEPARATOR : SEPARATOR) +
152
- getComponentKey(child, i)
150
+ (nameSoFar !== '' ? nameSoFar + SUBSEPARATOR : SEPARATOR) +
151
+ getComponentKey(child, ii++)
152
);
153
nextIndex = indexSoFar + subtreeCount;
154
subtreeCount += traverseAllChildrenImpl(
@@ -167,7 +166,7 @@ function traverseAllChildrenImpl(
166
if (entry) {
167
child = entry[1];
168
nextName = (
170
- nameSoFar + (nameSoFar ? SUBSEPARATOR : SEPARATOR) +
169
+ (nameSoFar !== '' ? nameSoFar + SUBSEPARATOR : SEPARATOR) +
170
wrapUserProvidedKey(entry[0]) + SUBSEPARATOR +
171
getComponentKey(child, 0)
172
);
@@ -192,7 +191,7 @@ function traverseAllChildrenImpl(
191
if (children.hasOwnProperty(key)) {
192
child = children[key];
193
nextName = (
195
- nameSoFar + (nameSoFar ? SUBSEPARATOR : SEPARATOR) +
194
+ (nameSoFar !== '' ? nameSoFar + SUBSEPARATOR : SEPARATOR) +
195
wrapUserProvidedKey(key) + SUBSEPARATOR +
196
getComponentKey(child, 0)
197
);