Skip to content

Commit

Permalink
Merge pull request #3651 from spicyj/tac-noi
Browse files Browse the repository at this point in the history 
Don't thread index through traverseAllChildren
  • Loading branch information
@sophiebits
sophiebits committed May 14, 2015
2 parents 05b98ac + ce21548 commit 5c7b4a0
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 84 deletions.
29 changes: 15 additions & 14 deletions src/utils/ReactChildren.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ var threeArgumentPooler = PooledClass.threeArgumentPooler;
* @param {?*} forEachContext Context to perform context with.
*/
function ForEachBookKeeping(forEachFunction, forEachContext) {
this.forEachFunction = forEachFunction;
this.forEachContext = forEachContext;
this.func = forEachFunction;
this.context = forEachContext;
this.count = 0;
}
PooledClass.addPoolingTo(ForEachBookKeeping, twoArgumentPooler);

function forEachSingleChild(traverseContext, child, name, i) {
var forEachBookKeeping = traverseContext;
forEachBookKeeping.forEachFunction.call(
forEachBookKeeping.forEachContext, child, i);
function forEachSingleChild(traverseContext, child, name) {
var bookKeeping = traverseContext;
bookKeeping.func.call(bookKeeping.context, child, bookKeeping.count++);
}

/**
Expand Down Expand Up @@ -71,15 +71,16 @@ function forEachChildren(children, forEachFunc, forEachContext) {
* @param {?*} mapContext Context to perform mapping with.
*/
function MapBookKeeping(mapResult, mapFunction, mapContext) {
this.mapResult = mapResult;
this.mapFunction = mapFunction;
this.mapContext = mapContext;
this.result = mapResult;
this.func = mapFunction;
this.context = mapContext;
this.count = 0;
}
PooledClass.addPoolingTo(MapBookKeeping, threeArgumentPooler);

function mapSingleChildIntoContext(traverseContext, child, name, i) {
var mapBookKeeping = traverseContext;
var mapResult = mapBookKeeping.mapResult;
function mapSingleChildIntoContext(traverseContext, child, name) {
var bookKeeping = traverseContext;
var mapResult = bookKeeping.result;

var keyUnique = (mapResult[name] === undefined);
if (__DEV__) {
Expand All @@ -94,7 +95,7 @@ function mapSingleChildIntoContext(traverseContext, child, name, i) {

if (keyUnique) {
var mappedChild =
mapBookKeeping.mapFunction.call(mapBookKeeping.mapContext, child, i);
bookKeeping.func.call(bookKeeping.context, child, bookKeeping.count++);
mapResult[name] = mappedChild;
}
}
Expand Down Expand Up @@ -125,7 +126,7 @@ function mapChildren(children, func, context) {
return ReactFragment.create(mapResult);
}

function forEachSingleChildDummy(traverseContext, child, name, i) {
function forEachSingleChildDummy(traverseContext, child, name) {
return null;
}

Expand Down
90 changes: 34 additions & 56 deletions src/utils/__tests__/traverseAllChildren-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ describe('traverseAllChildren', function() {
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
simpleKid,
'.$simple',
0
'.$simple'
);
expect(traverseContext.length).toEqual(1);
});
Expand All @@ -59,8 +58,7 @@ describe('traverseAllChildren', function() {
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
simpleKid,
'.0',
0
'.0'
);
expect(traverseContext.length).toEqual(1);
});
Expand All @@ -78,8 +76,7 @@ describe('traverseAllChildren', function() {
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
simpleKid,
'.0',
0
'.0'
);
expect(traverseContext.length).toEqual(1);
});
Expand Down Expand Up @@ -111,22 +108,19 @@ describe('traverseAllChildren', function() {
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
zero,
'.$keyZero',
0
'.$keyZero'
);
expect(traverseFn).toHaveBeenCalledWith(traverseContext, one, '.1', 1);
expect(traverseFn).toHaveBeenCalledWith(traverseContext, one, '.1');
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
two,
'.$keyTwo',
2
'.$keyTwo'
);
expect(traverseFn).toHaveBeenCalledWith(traverseContext, three, '.3', 3);
expect(traverseFn).toHaveBeenCalledWith(traverseContext, three, '.3');
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
four,
'.$keyFour',
4
'.$keyFour'
);
});

Expand Down Expand Up @@ -161,31 +155,31 @@ describe('traverseAllChildren', function() {
expect(traverseContext.length).toEqual(9);

expect(traverseFn).toHaveBeenCalledWith(
traverseContext, div, '.$divNode', 0
traverseContext, div, '.$divNode'
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext, span, '.1:0:$span:$spanNode', 1
traverseContext, span, '.1:0:$span:$spanNode'
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext, a, '.2:$a:$aNode', 2
traverseContext, a, '.2:$a:$aNode'
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext, 'string', '.3', 3
traverseContext, 'string', '.3'
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext, 1234, '.4', 4
traverseContext, 1234, '.4'
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext, null, '.5', 5
traverseContext, null, '.5'
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext, null, '.6', 6
traverseContext, null, '.6'
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext, null, '.7', 7
traverseContext, null, '.7'
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext, null, '.8', 8
traverseContext, null, '.8'
);
});

Expand Down Expand Up @@ -225,39 +219,34 @@ describe('traverseAllChildren', function() {
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
zero,
'.0:$firstHalfKey:0:$keyZero',
0
'.0:$firstHalfKey:0:$keyZero'
);

expect(traverseFn)
.toHaveBeenCalledWith(traverseContext, one, '.0:$firstHalfKey:0:1', 1);
.toHaveBeenCalledWith(traverseContext, one, '.0:$firstHalfKey:0:1');

expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
two,
'.0:$firstHalfKey:0:$keyTwo',
2
'.0:$firstHalfKey:0:$keyTwo'
);

expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
three,
'.0:$secondHalfKey:0:0',
3
'.0:$secondHalfKey:0:0'
);

expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
four,
'.0:$secondHalfKey:0:$keyFour',
4
'.0:$secondHalfKey:0:$keyFour'
);

expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
five,
'.0:$keyFive:$keyFiveInner',
5
'.0:$keyFive:$keyFiveInner'
);
});

Expand All @@ -282,14 +271,12 @@ describe('traverseAllChildren', function() {
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
zeroForceKey,
'.$keyZero',
0
'.$keyZero'
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
oneForceKey,
'.$keyOne',
1
'.$keyOne'
);
});

Expand Down Expand Up @@ -327,20 +314,17 @@ describe('traverseAllChildren', function() {
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
traverseContext[0],
'.0',
0
'.0'
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
traverseContext[1],
'.1',
1
'.1'
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
traverseContext[2],
'.2',
2
'.2'
);
});

Expand Down Expand Up @@ -378,20 +362,17 @@ describe('traverseAllChildren', function() {
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
traverseContext[0],
'.$#1',
0
'.$#1'
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
traverseContext[1],
'.$#2',
1
'.$#2'
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
traverseContext[2],
'.$#3',
2
'.$#3'
);
});

Expand Down Expand Up @@ -432,20 +413,17 @@ describe('traverseAllChildren', function() {
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
traverseContext[0],
'.$#1:0',
0
'.$#1:0'
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
traverseContext[1],
'.$#2:0',
1
'.$#2:0'
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
traverseContext[2],
'.$#3:0',
2
'.$#3:0'
);

expect(console.error.argsForCall.length).toBe(1);
Expand Down
18 changes: 4 additions & 14 deletions src/utils/traverseAllChildren.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ function wrapUserProvidedKey(key) {
/**
* @param {?*} children Children tree container.
* @param {!string} nameSoFar Name of the key path so far.
* @param {!number} indexSoFar Number of children encountered until this point.
* @param {!function} callback Callback to invoke with each child found.
* @param {?*} traverseContext Used to pass information throughout the traversal
* process.
Expand All @@ -93,7 +92,6 @@ function wrapUserProvidedKey(key) {
function traverseAllChildrenImpl(
children,
nameSoFar,
indexSoFar,
callback,
traverseContext
) {
Expand All @@ -113,13 +111,13 @@ function traverseAllChildrenImpl(
children,
// If it's the only child, treat the name as if it was wrapped in an array
// so that it's consistent if the number of children grows.
nameSoFar === '' ? SEPARATOR + getComponentKey(children, 0) : nameSoFar,
indexSoFar
nameSoFar === '' ? SEPARATOR + getComponentKey(children, 0) : nameSoFar
);
return 1;
}

var child, nextName, nextIndex;
var child;
var nextName;
var subtreeCount = 0; // Count of children found in the current subtree.

if (Array.isArray(children)) {
Expand All @@ -129,11 +127,9 @@ function traverseAllChildrenImpl(
(nameSoFar !== '' ? nameSoFar + SUBSEPARATOR : SEPARATOR) +
getComponentKey(child, i)
);
nextIndex = indexSoFar + subtreeCount;
subtreeCount += traverseAllChildrenImpl(
child,
nextName,
nextIndex,
callback,
traverseContext
);
Expand All @@ -151,11 +147,9 @@ function traverseAllChildrenImpl(
(nameSoFar !== '' ? nameSoFar + SUBSEPARATOR : SEPARATOR) +
getComponentKey(child, ii++)
);
nextIndex = indexSoFar + subtreeCount;
subtreeCount += traverseAllChildrenImpl(
child,
nextName,
nextIndex,
callback,
traverseContext
);
Expand All @@ -180,11 +174,9 @@ function traverseAllChildrenImpl(
wrapUserProvidedKey(entry[0]) + SUBSEPARATOR +
getComponentKey(child, 0)
);
nextIndex = indexSoFar + subtreeCount;
subtreeCount += traverseAllChildrenImpl(
child,
nextName,
nextIndex,
callback,
traverseContext
);
Expand All @@ -206,11 +198,9 @@ function traverseAllChildrenImpl(
wrapUserProvidedKey(key) + SUBSEPARATOR +
getComponentKey(child, 0)
);
nextIndex = indexSoFar + subtreeCount;
subtreeCount += traverseAllChildrenImpl(
child,
nextName,
nextIndex,
callback,
traverseContext
);
Expand Down Expand Up @@ -243,7 +233,7 @@ function traverseAllChildren(children, callback, traverseContext) {
return 0;
}

return traverseAllChildrenImpl(children, '', 0, callback, traverseContext);
return traverseAllChildrenImpl(children, '', callback, traverseContext);
}

module.exports = traverseAllChildren;

0 comments on commit 5c7b4a0

Please sign in to comment.