From 9ef4e74ba2b3a494757b39b41b1ef99280bc5f83 Mon Sep 17 00:00:00 2001 From: Paul Shen Date: Tue, 6 Aug 2013 14:14:44 -0700 Subject: [PATCH] ReactChildren Instead of changing `traverseAllChildren`, keep that around for perf reasons (for the hot code path `flattenChildren`) Introduce `ReactChildren.map` and `ReactChildren.forEach` which mirrors `Array.prototype.map` and `Array.prototype.forEach`. This involves a rename of `mapAllChildren` --- src/core/React.js | 3 + .../{mapAllChildren.js => ReactChildren.js} | 74 +++++++++--- ...Children-test.js => ReactChildren-test.js} | 108 ++++++++++++------ 3 files changed, 131 insertions(+), 54 deletions(-) rename src/utils/{mapAllChildren.js => ReactChildren.js} (53%) rename src/utils/__tests__/{mapAllChildren-test.js => ReactChildren-test.js} (67%) diff --git a/src/core/React.js b/src/core/React.js index 1d7d9c3a0e..9451c726f3 100644 --- a/src/core/React.js +++ b/src/core/React.js @@ -18,6 +18,7 @@ "use strict"; +var ReactChildren = require('ReactChildren'); var ReactCompositeComponent = require('ReactCompositeComponent'); var ReactComponent = require('ReactComponent'); var ReactDOM = require('ReactDOM'); @@ -39,6 +40,8 @@ var React = { createClass: ReactCompositeComponent.createClass, constructAndRenderComponent: ReactMount.constructAndRenderComponent, constructAndRenderComponentByID: ReactMount.constructAndRenderComponentByID, + forEachChildren: ReactChildren.forEach, + mapChildren: ReactChildren.map, renderComponent: ReactMount.renderComponent, renderComponentToString: ReactServerRendering.renderComponentToString, unmountAndReleaseReactRootNode: ReactMount.unmountAndReleaseReactRootNode, diff --git a/src/utils/mapAllChildren.js b/src/utils/ReactChildren.js similarity index 53% rename from src/utils/mapAllChildren.js rename to src/utils/ReactChildren.js index 9eff484798..f2b42a77f0 100644 --- a/src/utils/mapAllChildren.js +++ b/src/utils/ReactChildren.js @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. * - * @providesModule mapAllChildren + * @providesModule ReactChildren */ "use strict"; @@ -23,8 +23,50 @@ var PooledClass = require('PooledClass'); var invariant = require('invariant'); var traverseAllChildren = require('traverseAllChildren'); +var twoArgumentPooler = PooledClass.twoArgumentPooler; var threeArgumentPooler = PooledClass.threeArgumentPooler; +/** + * PooledClass representing the bookkeeping associated with performing a child + * traversal. Allows avoiding binding callbacks. + * + * @constructor ForEachBookKeeping + * @param {!function} forEachFunction Function to perform traversal with. + * @param {?*} forEachContext Context to perform context with. + */ +function ForEachBookKeeping(forEachFunction, forEachContext) { + this.forEachFunction = forEachFunction; + this.forEachContext = forEachContext; +} +PooledClass.addPoolingTo(ForEachBookKeeping, twoArgumentPooler); + +function forEachSingleChild(traverseContext, child, name, i) { + var forEachBookKeeping = traverseContext; + forEachBookKeeping.forEachFunction.call( + forEachBookKeeping.forEachContext, child, i); +} + +/** + * Iterates through children that are typically specified as `props.children`. + * + * The provided forEachFunc(child, index) will be called for each + * leaf child. + * + * @param {array} children + * @param {function(*, int)} forEachFunc. + * @param {*} forEachContext Context for forEachContext. + */ +function forEachChildren(children, forEachFunc, forEachContext) { + if (children == null) { + return children; + } + + var traverseContext = + ForEachBookKeeping.getPooled(forEachFunc, forEachContext); + traverseAllChildren(children, forEachSingleChild, traverseContext); + ForEachBookKeeping.release(traverseContext); +} + /** * PooledClass representing the bookkeeping associated with performing a child * mapping. Allows avoiding binding callbacks. @@ -41,20 +83,16 @@ function MapBookKeeping(mapResult, mapFunction, mapContext) { } PooledClass.addPoolingTo(MapBookKeeping, threeArgumentPooler); -module.exports = MapBookKeeping; - - function mapSingleChildIntoContext(traverseContext, child, name, i) { var mapBookKeeping = traverseContext; var mapResult = mapBookKeeping.mapResult; - var mapFunction = mapBookKeeping.mapFunction; - var mapContext = mapBookKeeping.mapContext; - var mappedChild = mapFunction.call(mapContext, child, name, i); - // We found a component instance. + var mappedChild = + mapBookKeeping.mapFunction.call(mapBookKeeping.mapContext, child, i); + // We found a component instance invariant( !mapResult.hasOwnProperty(name), - 'mapAllChildren(...): Encountered two children with the same key, `%s`. ' + - 'Children keys must be unique.', + 'ReactChildren.map(...): Encountered two children with the same key, ' + + '`%s`. Children keys must be unique.', name ); mapResult[name] = mappedChild; @@ -66,25 +104,29 @@ function mapSingleChildIntoContext(traverseContext, child, name, i) { * The provided mapFunction(child, key, index) will be called for each * leaf child. * - * TODO: This may likely break any calls to `mapAllChildren` that were + * TODO: This may likely break any calls to `ReactChildren.map` that were * previously relying on the fact that we guarded against null children. * * @param {array} children - * @param {function(*, string, int)} mapFunction. + * @param {function(*, int)} mapFunction. * @param {*} mapContext Context for mapFunction. * @return {array} mirrored array with mapped children. */ -function mapAllChildren(children, mapFunction, mapContext) { +function mapChildren(children, func, context) { if (children == null) { return children; } var mapResult = {}; - var traverseContext = - MapBookKeeping.getPooled(mapResult, mapFunction, mapContext); + var traverseContext = MapBookKeeping.getPooled(mapResult, func, context); traverseAllChildren(children, mapSingleChildIntoContext, traverseContext); MapBookKeeping.release(traverseContext); return mapResult; } -module.exports = mapAllChildren; +var ReactChildren = { + forEach: forEachChildren, + map: mapChildren +}; + +module.exports = ReactChildren; diff --git a/src/utils/__tests__/mapAllChildren-test.js b/src/utils/__tests__/ReactChildren-test.js similarity index 67% rename from src/utils/__tests__/mapAllChildren-test.js rename to src/utils/__tests__/ReactChildren-test.js index ac9a530dd3..6c684b7d2a 100644 --- a/src/utils/__tests__/mapAllChildren-test.js +++ b/src/utils/__tests__/ReactChildren-test.js @@ -19,18 +19,18 @@ "use strict"; -describe('mapAllChildren', function() { - var mapAllChildren; +describe('ReactChildren', function() { + var ReactChildren; var React; beforeEach(function() { - mapAllChildren = require('mapAllChildren'); + ReactChildren = require('ReactChildren'); React = require('React'); }); it('should support identity for simple', function() { - var mapFn = jasmine.createSpy().andCallFake(function (kid, key, index) { + var callback = jasmine.createSpy().andCallFake(function (kid, index) { return kid; }); @@ -40,44 +40,53 @@ describe('mapAllChildren', function() { // using structures that arrive from transforms. var instance =
{simpleKid}
; - var mappedChildren = mapAllChildren(instance.props.children, mapFn); - expect(mapFn).toHaveBeenCalledWith(simpleKid, '[simple]', 0); + ReactChildren.forEach(instance.props.children, callback); + expect(callback).toHaveBeenCalledWith(simpleKid, 0); + callback.reset(); + var mappedChildren = ReactChildren.map(instance.props.children, callback); + expect(callback).toHaveBeenCalledWith(simpleKid, 0); expect(mappedChildren[Object.keys(mappedChildren)[0]]).toBe(simpleKid); }); it('should treat single arrayless child as being in array', function() { - var mapFn = jasmine.createSpy().andCallFake(function (kid, key, index) { + var callback = jasmine.createSpy().andCallFake(function (kid, index) { return kid; }); var simpleKid = ; var instance =
{simpleKid}
; - var mappedChildren = mapAllChildren(instance.props.children, mapFn); - expect(mapFn).toHaveBeenCalledWith(simpleKid, '[0]', 0); + ReactChildren.forEach(instance.props.children, callback); + expect(callback).toHaveBeenCalledWith(simpleKid, 0); + callback.reset(); + var mappedChildren = ReactChildren.map(instance.props.children, callback); + expect(callback).toHaveBeenCalledWith(simpleKid, 0); expect(mappedChildren[Object.keys(mappedChildren)[0]]).toBe(simpleKid); }); it('should treat single child in array as expected', function() { - var mapFn = jasmine.createSpy().andCallFake(function (kid, key, index) { + var callback = jasmine.createSpy().andCallFake(function (kid, index) { return kid; }); var simpleKid = ; var instance =
{[simpleKid]}
; - var mappedChildren = mapAllChildren(instance.props.children, mapFn); - expect(mapFn).toHaveBeenCalledWith(simpleKid, '[0]', 0); + ReactChildren.forEach(instance.props.children, callback); + expect(callback).toHaveBeenCalledWith(simpleKid, 0); + callback.reset(); + var mappedChildren = ReactChildren.map(instance.props.children, callback); + expect(callback).toHaveBeenCalledWith(simpleKid, 0); expect(mappedChildren[Object.keys(mappedChildren)[0]]).toBe(simpleKid); }); it('should pass key to returned component', function() { - var mapFn = function (kid, key, index) { + var mapFn = function (kid, index) { return
{kid}
; }; var simpleKid = ; var instance =
{simpleKid}
; - var mappedChildren = mapAllChildren(instance.props.children, mapFn); + var mappedChildren = ReactChildren.map(instance.props.children, mapFn); var mappedKeys = Object.keys(mappedChildren); expect(mappedKeys.length).toBe(1); @@ -87,7 +96,9 @@ describe('mapAllChildren', function() { }); it('should invoke callback with the right context', function() { - var mapFn = function (kid, key, index) { + var lastContext; + var callback = function (kid, index) { + lastContext = this; return this; }; @@ -95,8 +106,11 @@ describe('mapAllChildren', function() { var simpleKid = ; var instance =
{simpleKid}
; + ReactChildren.forEach(instance.props.children, callback, scopeTester); + expect(lastContext).toBe(scopeTester); + var mappedChildren = - mapAllChildren(instance.props.children, mapFn, scopeTester); + ReactChildren.map(instance.props.children, callback, scopeTester); var mappedKeys = Object.keys(mappedChildren); expect(mappedKeys.length).toBe(1); @@ -116,7 +130,7 @@ describe('mapAllChildren', function() { var threeMapped = ; // Map from null to something. var fourMapped =
; - var mapFn = jasmine.createSpy().andCallFake(function (kid, key, index) { + var callback = jasmine.createSpy().andCallFake(function (kid, index) { return index === 0 ? zeroMapped : index === 1 ? oneMapped : index === 2 ? twoMapped : @@ -133,28 +147,37 @@ describe('mapAllChildren', function() {
); - var mappedChildren = mapAllChildren(instance.props.children, mapFn); + ReactChildren.forEach(instance.props.children, callback); + expect(callback).toHaveBeenCalledWith(zero, 0); + expect(callback).toHaveBeenCalledWith(one, 1); + expect(callback).toHaveBeenCalledWith(two, 2); + expect(callback).toHaveBeenCalledWith(three, 3); + expect(callback).toHaveBeenCalledWith(four, 4); + callback.reset(); + + var mappedChildren = + ReactChildren.map(instance.props.children, callback); var mappedKeys = Object.keys(mappedChildren); - expect(mapFn.calls.length).toBe(5); + expect(callback.calls.length).toBe(5); expect(mappedKeys.length).toBe(5); // Keys default to indices. expect(mappedKeys).toEqual( ['[keyZero]', '[1]', '[keyTwo]', '[3]', '[keyFour]'] ); - expect(mapFn).toHaveBeenCalledWith(zero, '[keyZero]', 0); + expect(callback).toHaveBeenCalledWith(zero, 0); expect(mappedChildren[mappedKeys[0]]).toBe(zeroMapped); - expect(mapFn).toHaveBeenCalledWith(one, '[1]', 1); + expect(callback).toHaveBeenCalledWith(one, 1); expect(mappedChildren[mappedKeys[1]]).toBe(oneMapped); - expect(mapFn).toHaveBeenCalledWith(two, '[keyTwo]', 2); + expect(callback).toHaveBeenCalledWith(two, 2); expect(mappedChildren[mappedKeys[2]]).toBe(twoMapped); - expect(mapFn).toHaveBeenCalledWith(three, '[3]', 3); + expect(callback).toHaveBeenCalledWith(three, 3); expect(mappedChildren[mappedKeys[3]]).toBe(threeMapped); - expect(mapFn).toHaveBeenCalledWith(four, '[keyFour]', 4); + expect(callback).toHaveBeenCalledWith(four, 4); expect(mappedChildren[mappedKeys[4]]).toBe(fourMapped); }); @@ -180,7 +203,7 @@ describe('mapAllChildren', function() { var fourMapped =
; var fiveMapped =
; - var mapFn = jasmine.createSpy().andCallFake(function (kid, key, index) { + var callback = jasmine.createSpy().andCallFake(function (kid, index) { return index === 0 ? zeroMapped : index === 1 ? oneMapped : index === 2 ? twoMapped : @@ -198,9 +221,18 @@ describe('mapAllChildren', function() { }
); - var mappedChildren = mapAllChildren(instance.props.children, mapFn); + ReactChildren.forEach(instance.props.children, callback); + expect(callback).toHaveBeenCalledWith(zero, 0); + expect(callback).toHaveBeenCalledWith(one, 1); + expect(callback).toHaveBeenCalledWith(two, 2); + expect(callback).toHaveBeenCalledWith(three, 3); + expect(callback).toHaveBeenCalledWith(four, 4); + expect(callback).toHaveBeenCalledWith(five, 5); + callback.reset(); + + var mappedChildren = ReactChildren.map(instance.props.children, callback); var mappedKeys = Object.keys(mappedChildren); - expect(mapFn.calls.length).toBe(6); + expect(callback.calls.length).toBe(6); expect(mappedKeys.length).toBe(6); // Keys default to indices. expect(mappedKeys).toEqual([ @@ -212,22 +244,22 @@ describe('mapAllChildren', function() { '[0]{keyFive}' ]); - expect(mapFn).toHaveBeenCalledWith(zero, '[0]{firstHalfKey}[keyZero]', 0); + expect(callback).toHaveBeenCalledWith(zero, 0); expect(mappedChildren[mappedKeys[0]]).toBe(zeroMapped); - expect(mapFn).toHaveBeenCalledWith(one, '[0]{firstHalfKey}[1]', 1); + expect(callback).toHaveBeenCalledWith(one, 1); expect(mappedChildren[mappedKeys[1]]).toBe(oneMapped); - expect(mapFn).toHaveBeenCalledWith(two, '[0]{firstHalfKey}[keyTwo]', 2); + expect(callback).toHaveBeenCalledWith(two, 2); expect(mappedChildren[mappedKeys[2]]).toBe(twoMapped); - expect(mapFn).toHaveBeenCalledWith(three, '[0]{secondHalfKey}[0]', 3); + expect(callback).toHaveBeenCalledWith(three, 3); expect(mappedChildren[mappedKeys[3]]).toBe(threeMapped); - expect(mapFn).toHaveBeenCalledWith(four, '[0]{secondHalfKey}[keyFour]', 4); + expect(callback).toHaveBeenCalledWith(four, 4); expect(mappedChildren[mappedKeys[4]]).toBe(fourMapped); - expect(mapFn).toHaveBeenCalledWith(five, '[0]{keyFive}', 5); + expect(callback).toHaveBeenCalledWith(five, 5); expect(mappedChildren[mappedKeys[5]]).toBe(fiveMapped); }); @@ -240,7 +272,7 @@ describe('mapAllChildren', function() { // Key should be added even if we don't supply it! var oneForceKeyMapped =
; - var mapFn = function(kid, key, index) { + var mapFn = function(kid, index) { return index === 0 ? zeroForceKeyMapped : oneForceKeyMapped; }; @@ -253,13 +285,13 @@ describe('mapAllChildren', function() { var expectedForcedKeys = ['[keyZero]', '[keyOne]']; var mappedChildrenForcedKeys = - mapAllChildren(forcedKeys.props.children, mapFn); + ReactChildren.map(forcedKeys.props.children, mapFn); var mappedForcedKeys = Object.keys(mappedChildrenForcedKeys); expect(mappedForcedKeys).toEqual(expectedForcedKeys); var expectedRemappedForcedKeys = ['{[keyZero]}', '{[keyOne]}']; var remappedChildrenForcedKeys = - mapAllChildren(mappedChildrenForcedKeys, mapFn); + ReactChildren.map(mappedChildrenForcedKeys, mapFn); expect( Object.keys(remappedChildrenForcedKeys) ).toEqual(expectedRemappedForcedKeys); @@ -282,7 +314,7 @@ describe('mapAllChildren', function() { ); expect(function() { - mapAllChildren(instance.props.children, mapFn); + ReactChildren.map(instance.props.children, mapFn); }).toThrow(); }); @@ -296,7 +328,7 @@ describe('mapAllChildren', function() { ); expect(function() { - mapAllChildren(instance.props.children, mapFn); + ReactChildren.map(instance.props.children, mapFn); }).toThrow(); }); });