@samitouri / QOS-React-2 / commits / 9ef4e74ba2

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`

Paul Shen committed Aug 6, 2013 at 14:14 UTC 9ef4e74ba2b3a494757b39b41b1ef99280bc5f83
3 files changed +131 -54
src/core/React.js
+3
@@ -18,6 +18,7 @@
18
19 "use strict";
20
21 +var ReactChildren = require('ReactChildren');
22 var ReactCompositeComponent = require('ReactCompositeComponent');
23 var ReactComponent = require('ReactComponent');
24 var ReactDOM = require('ReactDOM');
@@ -39,6 +40,8 @@ var React = {
40 createClass: ReactCompositeComponent.createClass,
41 constructAndRenderComponent: ReactMount.constructAndRenderComponent,
42 constructAndRenderComponentByID: ReactMount.constructAndRenderComponentByID,
43 + forEachChildren: ReactChildren.forEach,
44 + mapChildren: ReactChildren.map,
45 renderComponent: ReactMount.renderComponent,
46 renderComponentToString: ReactServerRendering.renderComponentToString,
47 unmountAndReleaseReactRootNode: ReactMount.unmountAndReleaseReactRootNode,
src/utils/ReactChildren.js renamed
+58 -16
@@ -13,7 +13,7 @@
13 * See the License for the specific language governing permissions and
14 * limitations under the License.
15 *
16 - * @providesModule mapAllChildren
16 + * @providesModule ReactChildren
17 */
18
19 "use strict";
@@ -23,8 +23,50 @@ var PooledClass = require('PooledClass');
23 var invariant = require('invariant');
24 var traverseAllChildren = require('traverseAllChildren');
25
26 +var twoArgumentPooler = PooledClass.twoArgumentPooler;
27 var threeArgumentPooler = PooledClass.threeArgumentPooler;
28
29 +/**
30 + * PooledClass representing the bookkeeping associated with performing a child
31 + * traversal. Allows avoiding binding callbacks.
32 + *
33 + * @constructor ForEachBookKeeping
34 + * @param {!function} forEachFunction Function to perform traversal with.
35 + * @param {?*} forEachContext Context to perform context with.
36 + */
37 +function ForEachBookKeeping(forEachFunction, forEachContext) {
38 + this.forEachFunction = forEachFunction;
39 + this.forEachContext = forEachContext;
40 +}
41 +PooledClass.addPoolingTo(ForEachBookKeeping, twoArgumentPooler);
42 +
43 +function forEachSingleChild(traverseContext, child, name, i) {
44 + var forEachBookKeeping = traverseContext;
45 + forEachBookKeeping.forEachFunction.call(
46 + forEachBookKeeping.forEachContext, child, i);
47 +}
48 +
49 +/**
50 + * Iterates through children that are typically specified as `props.children`.
51 + *
52 + * The provided forEachFunc(child, index) will be called for each
53 + * leaf child.
54 + *
55 + * @param {array} children
56 + * @param {function(*, int)} forEachFunc.
57 + * @param {*} forEachContext Context for forEachContext.
58 + */
59 +function forEachChildren(children, forEachFunc, forEachContext) {
60 + if (children == null) {
61 + return children;
62 + }
63 +
64 + var traverseContext =
65 + ForEachBookKeeping.getPooled(forEachFunc, forEachContext);
66 + traverseAllChildren(children, forEachSingleChild, traverseContext);
67 + ForEachBookKeeping.release(traverseContext);
68 +}
69 +
70 /**
71 * PooledClass representing the bookkeeping associated with performing a child
72 * mapping. Allows avoiding binding callbacks.
@@ -41,20 +83,16 @@ function MapBookKeeping(mapResult, mapFunction, mapContext) {
83 }
84 PooledClass.addPoolingTo(MapBookKeeping, threeArgumentPooler);
85
44 -module.exports = MapBookKeeping;
45 -
46 -
86 function mapSingleChildIntoContext(traverseContext, child, name, i) {
87 var mapBookKeeping = traverseContext;
88 var mapResult = mapBookKeeping.mapResult;
50 - var mapFunction = mapBookKeeping.mapFunction;
51 - var mapContext = mapBookKeeping.mapContext;
52 - var mappedChild = mapFunction.call(mapContext, child, name, i);
53 - // We found a component instance.
89 + var mappedChild =
90 + mapBookKeeping.mapFunction.call(mapBookKeeping.mapContext, child, i);
91 + // We found a component instance
92 invariant(
93 !mapResult.hasOwnProperty(name),
56 - 'mapAllChildren(...): Encountered two children with the same key, `%s`. ' +
57 - 'Children keys must be unique.',
94 + 'ReactChildren.map(...): Encountered two children with the same key, ' +
95 + '`%s`. Children keys must be unique.',
96 name
97 );
98 mapResult[name] = mappedChild;
@@ -66,25 +104,29 @@ function mapSingleChildIntoContext(traverseContext, child, name, i) {
104 * The provided mapFunction(child, key, index) will be called for each
105 * leaf child.
106 *
69 - * TODO: This may likely break any calls to `mapAllChildren` that were
107 + * TODO: This may likely break any calls to `ReactChildren.map` that were
108 * previously relying on the fact that we guarded against null children.
109 *
110 * @param {array} children
73 - * @param {function(*, string, int)} mapFunction.
111 + * @param {function(*, int)} mapFunction.
112 * @param {*} mapContext Context for mapFunction.
113 * @return {array} mirrored array with mapped children.
114 */
77 -function mapAllChildren(children, mapFunction, mapContext) {
115 +function mapChildren(children, func, context) {
116 if (children == null) {
117 return children;
118 }
119
120 var mapResult = {};
83 - var traverseContext =
84 - MapBookKeeping.getPooled(mapResult, mapFunction, mapContext);
121 + var traverseContext = MapBookKeeping.getPooled(mapResult, func, context);
122 traverseAllChildren(children, mapSingleChildIntoContext, traverseContext);
123 MapBookKeeping.release(traverseContext);
124 return mapResult;
125 }
126
90 -module.exports = mapAllChildren;
127 +var ReactChildren = {
128 + forEach: forEachChildren,
129 + map: mapChildren
130 +};
131 +
132 +module.exports = ReactChildren;
src/utils/__tests__/ReactChildren-test.js renamed
+70 -38
@@ -19,18 +19,18 @@
19
20 "use strict";
21
22 -describe('mapAllChildren', function() {
23 - var mapAllChildren;
22 +describe('ReactChildren', function() {
23 + var ReactChildren;
24 var React;
25
26 beforeEach(function() {
27 - mapAllChildren = require('mapAllChildren');
27 + ReactChildren = require('ReactChildren');
28 React = require('React');
29 });
30
31
32 it('should support identity for simple', function() {
33 - var mapFn = jasmine.createSpy().andCallFake(function (kid, key, index) {
33 + var callback = jasmine.createSpy().andCallFake(function (kid, index) {
34 return kid;
35 });
36
@@ -40,44 +40,53 @@ describe('mapAllChildren', function() {
40 // using structures that arrive from transforms.
41
42 var instance = <div>{simpleKid}</div>;
43 - var mappedChildren = mapAllChildren(instance.props.children, mapFn);
44 - expect(mapFn).toHaveBeenCalledWith(simpleKid, '[simple]', 0);
43 + ReactChildren.forEach(instance.props.children, callback);
44 + expect(callback).toHaveBeenCalledWith(simpleKid, 0);
45 + callback.reset();
46 + var mappedChildren = ReactChildren.map(instance.props.children, callback);
47 + expect(callback).toHaveBeenCalledWith(simpleKid, 0);
48 expect(mappedChildren[Object.keys(mappedChildren)[0]]).toBe(simpleKid);
49 });
50
51 it('should treat single arrayless child as being in array', function() {
49 - var mapFn = jasmine.createSpy().andCallFake(function (kid, key, index) {
52 + var callback = jasmine.createSpy().andCallFake(function (kid, index) {
53 return kid;
54 });
55
56 var simpleKid = <span />;
57 var instance = <div>{simpleKid}</div>;
55 - var mappedChildren = mapAllChildren(instance.props.children, mapFn);
56 - expect(mapFn).toHaveBeenCalledWith(simpleKid, '[0]', 0);
58 + ReactChildren.forEach(instance.props.children, callback);
59 + expect(callback).toHaveBeenCalledWith(simpleKid, 0);
60 + callback.reset();
61 + var mappedChildren = ReactChildren.map(instance.props.children, callback);
62 + expect(callback).toHaveBeenCalledWith(simpleKid, 0);
63 expect(mappedChildren[Object.keys(mappedChildren)[0]]).toBe(simpleKid);
64 });
65
66 it('should treat single child in array as expected', function() {
61 - var mapFn = jasmine.createSpy().andCallFake(function (kid, key, index) {
67 + var callback = jasmine.createSpy().andCallFake(function (kid, index) {
68 return kid;
69 });
70
71 var simpleKid = <span />;
72 var instance = <div>{[simpleKid]}</div>;
67 - var mappedChildren = mapAllChildren(instance.props.children, mapFn);
68 - expect(mapFn).toHaveBeenCalledWith(simpleKid, '[0]', 0);
73 + ReactChildren.forEach(instance.props.children, callback);
74 + expect(callback).toHaveBeenCalledWith(simpleKid, 0);
75 + callback.reset();
76 + var mappedChildren = ReactChildren.map(instance.props.children, callback);
77 + expect(callback).toHaveBeenCalledWith(simpleKid, 0);
78 expect(mappedChildren[Object.keys(mappedChildren)[0]]).toBe(simpleKid);
79 });
80
81 it('should pass key to returned component', function() {
73 - var mapFn = function (kid, key, index) {
82 + var mapFn = function (kid, index) {
83 return <div>{kid}</div>;
84 };
85
86 var simpleKid = <span key="simple" />;
87
88 var instance = <div>{simpleKid}</div>;
80 - var mappedChildren = mapAllChildren(instance.props.children, mapFn);
89 + var mappedChildren = ReactChildren.map(instance.props.children, mapFn);
90
91 var mappedKeys = Object.keys(mappedChildren);
92 expect(mappedKeys.length).toBe(1);
@@ -87,7 +96,9 @@ describe('mapAllChildren', function() {
96 });
97
98 it('should invoke callback with the right context', function() {
90 - var mapFn = function (kid, key, index) {
99 + var lastContext;
100 + var callback = function (kid, index) {
101 + lastContext = this;
102 return this;
103 };
104
@@ -95,8 +106,11 @@ describe('mapAllChildren', function() {
106
107 var simpleKid = <span key="simple" />;
108 var instance = <div>{simpleKid}</div>;
109 + ReactChildren.forEach(instance.props.children, callback, scopeTester);
110 + expect(lastContext).toBe(scopeTester);
111 +
112 var mappedChildren =
99 - mapAllChildren(instance.props.children, mapFn, scopeTester);
113 + ReactChildren.map(instance.props.children, callback, scopeTester);
114
115 var mappedKeys = Object.keys(mappedChildren);
116 expect(mappedKeys.length).toBe(1);
@@ -116,7 +130,7 @@ describe('mapAllChildren', function() {
130 var threeMapped = <span />; // Map from null to something.
131 var fourMapped = <div key="keyFour" />;
132
119 - var mapFn = jasmine.createSpy().andCallFake(function (kid, key, index) {
133 + var callback = jasmine.createSpy().andCallFake(function (kid, index) {
134 return index === 0 ? zeroMapped :
135 index === 1 ? oneMapped :
136 index === 2 ? twoMapped :
@@ -133,28 +147,37 @@ describe('mapAllChildren', function() {
147 </div>
148 );
149
136 - var mappedChildren = mapAllChildren(instance.props.children, mapFn);
150 + ReactChildren.forEach(instance.props.children, callback);
151 + expect(callback).toHaveBeenCalledWith(zero, 0);
152 + expect(callback).toHaveBeenCalledWith(one, 1);
153 + expect(callback).toHaveBeenCalledWith(two, 2);
154 + expect(callback).toHaveBeenCalledWith(three, 3);
155 + expect(callback).toHaveBeenCalledWith(four, 4);
156 + callback.reset();
157 +
158 + var mappedChildren =
159 + ReactChildren.map(instance.props.children, callback);
160 var mappedKeys = Object.keys(mappedChildren);
138 - expect(mapFn.calls.length).toBe(5);
161 + expect(callback.calls.length).toBe(5);
162 expect(mappedKeys.length).toBe(5);
163 // Keys default to indices.
164 expect(mappedKeys).toEqual(
165 ['[keyZero]', '[1]', '[keyTwo]', '[3]', '[keyFour]']
166 );
167
145 - expect(mapFn).toHaveBeenCalledWith(zero, '[keyZero]', 0);
168 + expect(callback).toHaveBeenCalledWith(zero, 0);
169 expect(mappedChildren[mappedKeys[0]]).toBe(zeroMapped);
170
148 - expect(mapFn).toHaveBeenCalledWith(one, '[1]', 1);
171 + expect(callback).toHaveBeenCalledWith(one, 1);
172 expect(mappedChildren[mappedKeys[1]]).toBe(oneMapped);
173
151 - expect(mapFn).toHaveBeenCalledWith(two, '[keyTwo]', 2);
174 + expect(callback).toHaveBeenCalledWith(two, 2);
175 expect(mappedChildren[mappedKeys[2]]).toBe(twoMapped);
176
154 - expect(mapFn).toHaveBeenCalledWith(three, '[3]', 3);
177 + expect(callback).toHaveBeenCalledWith(three, 3);
178 expect(mappedChildren[mappedKeys[3]]).toBe(threeMapped);
179
157 - expect(mapFn).toHaveBeenCalledWith(four, '[keyFour]', 4);
180 + expect(callback).toHaveBeenCalledWith(four, 4);
181 expect(mappedChildren[mappedKeys[4]]).toBe(fourMapped);
182 });
183
@@ -180,7 +203,7 @@ describe('mapAllChildren', function() {
203 var fourMapped = <div key="keyFour" />;
204 var fiveMapped = <div />;
205
183 - var mapFn = jasmine.createSpy().andCallFake(function (kid, key, index) {
206 + var callback = jasmine.createSpy().andCallFake(function (kid, index) {
207 return index === 0 ? zeroMapped :
208 index === 1 ? oneMapped :
209 index === 2 ? twoMapped :
@@ -198,9 +221,18 @@ describe('mapAllChildren', function() {
221 }</div>
222 );
223
201 - var mappedChildren = mapAllChildren(instance.props.children, mapFn);
224 + ReactChildren.forEach(instance.props.children, callback);
225 + expect(callback).toHaveBeenCalledWith(zero, 0);
226 + expect(callback).toHaveBeenCalledWith(one, 1);
227 + expect(callback).toHaveBeenCalledWith(two, 2);
228 + expect(callback).toHaveBeenCalledWith(three, 3);
229 + expect(callback).toHaveBeenCalledWith(four, 4);
230 + expect(callback).toHaveBeenCalledWith(five, 5);
231 + callback.reset();
232 +
233 + var mappedChildren = ReactChildren.map(instance.props.children, callback);
234 var mappedKeys = Object.keys(mappedChildren);
203 - expect(mapFn.calls.length).toBe(6);
235 + expect(callback.calls.length).toBe(6);
236 expect(mappedKeys.length).toBe(6);
237 // Keys default to indices.
238 expect(mappedKeys).toEqual([
@@ -212,22 +244,22 @@ describe('mapAllChildren', function() {
244 '[0]{keyFive}'
245 ]);
246
215 - expect(mapFn).toHaveBeenCalledWith(zero, '[0]{firstHalfKey}[keyZero]', 0);
247 + expect(callback).toHaveBeenCalledWith(zero, 0);
248 expect(mappedChildren[mappedKeys[0]]).toBe(zeroMapped);
249
218 - expect(mapFn).toHaveBeenCalledWith(one, '[0]{firstHalfKey}[1]', 1);
250 + expect(callback).toHaveBeenCalledWith(one, 1);
251 expect(mappedChildren[mappedKeys[1]]).toBe(oneMapped);
252
221 - expect(mapFn).toHaveBeenCalledWith(two, '[0]{firstHalfKey}[keyTwo]', 2);
253 + expect(callback).toHaveBeenCalledWith(two, 2);
254 expect(mappedChildren[mappedKeys[2]]).toBe(twoMapped);
255
224 - expect(mapFn).toHaveBeenCalledWith(three, '[0]{secondHalfKey}[0]', 3);
256 + expect(callback).toHaveBeenCalledWith(three, 3);
257 expect(mappedChildren[mappedKeys[3]]).toBe(threeMapped);
258
227 - expect(mapFn).toHaveBeenCalledWith(four, '[0]{secondHalfKey}[keyFour]', 4);
259 + expect(callback).toHaveBeenCalledWith(four, 4);
260 expect(mappedChildren[mappedKeys[4]]).toBe(fourMapped);
261
230 - expect(mapFn).toHaveBeenCalledWith(five, '[0]{keyFive}', 5);
262 + expect(callback).toHaveBeenCalledWith(five, 5);
263 expect(mappedChildren[mappedKeys[5]]).toBe(fiveMapped);
264 });
265
@@ -240,7 +272,7 @@ describe('mapAllChildren', function() {
272 // Key should be added even if we don't supply it!
273 var oneForceKeyMapped = <div />;
274
243 - var mapFn = function(kid, key, index) {
275 + var mapFn = function(kid, index) {
276 return index === 0 ? zeroForceKeyMapped : oneForceKeyMapped;
277 };
278
@@ -253,13 +285,13 @@ describe('mapAllChildren', function() {
285
286 var expectedForcedKeys = ['[keyZero]', '[keyOne]'];
287 var mappedChildrenForcedKeys =
256 - mapAllChildren(forcedKeys.props.children, mapFn);
288 + ReactChildren.map(forcedKeys.props.children, mapFn);
289 var mappedForcedKeys = Object.keys(mappedChildrenForcedKeys);
290 expect(mappedForcedKeys).toEqual(expectedForcedKeys);
291
292 var expectedRemappedForcedKeys = ['{[keyZero]}', '{[keyOne]}'];
293 var remappedChildrenForcedKeys =
262 - mapAllChildren(mappedChildrenForcedKeys, mapFn);
294 + ReactChildren.map(mappedChildrenForcedKeys, mapFn);
295 expect(
296 Object.keys(remappedChildrenForcedKeys)
297 ).toEqual(expectedRemappedForcedKeys);
@@ -282,7 +314,7 @@ describe('mapAllChildren', function() {
314 );
315
316 expect(function() {
285 - mapAllChildren(instance.props.children, mapFn);
317 + ReactChildren.map(instance.props.children, mapFn);
318 }).toThrow();
319 });
320
@@ -296,7 +328,7 @@ describe('mapAllChildren', function() {
328 );
329
330 expect(function() {
299 - mapAllChildren(instance.props.children, mapFn);
331 + ReactChildren.map(instance.props.children, mapFn);
332 }).toThrow();
333 });
334 });