Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Don't thread index through traverseAllChildren #3651

Merged
merged 1 commit into from
May 14, 2015
Merged

Don't thread index through traverseAllChildren #3651

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 15 additions & 14 deletions src/utils/ReactChildren.js
View file
Open in desktop
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.hasOwnProperty(name);
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
Open in desktop
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
Open in desktop
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;