Skip to content

Commit

Permalink
Merge pull request #2376 from leebyron/iterable
Browse files Browse the repository at this point in the history
Allow iterables in traverseAllChildren
  • Loading branch information
leebyron committed Oct 23, 2014
2 parents de1dacd + c07ea0b commit d545238
Show file tree
Hide file tree
Showing 5 changed files with 427 additions and 57 deletions.
30 changes: 23 additions & 7 deletions src/core/ReactElementValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ var ReactElement = require('ReactElement');
var ReactPropTypeLocations = require('ReactPropTypeLocations');
var ReactCurrentOwner = require('ReactCurrentOwner');

var getIteratorFn = require('getIteratorFn');
var monitorCodeUse = require('monitorCodeUse');

/**
Expand Down Expand Up @@ -68,7 +69,7 @@ function validateExplicitKey(component, parentType) {

warnAndMonitorForKeyUse(
'react_key_warning',
'Each child in an array should have a unique "key" prop.',
'Each child in an array or iterator should have a unique "key" prop.',
component,
parentType
);
Expand Down Expand Up @@ -123,7 +124,9 @@ function warnAndMonitorForKeyUse(warningID, message, component, parentType) {
// property, it may be the creator of the child that's responsible for
// assigning it a key.
var childOwnerName = null;
if (component._owner && component._owner !== ReactCurrentOwner.current) {
if (component &&
component._owner &&
component._owner !== ReactCurrentOwner.current) {
// Name of the component that originally created this child.
childOwnerName = component._owner.constructor.displayName;

Expand Down Expand Up @@ -161,7 +164,6 @@ function monitorUseOfObjectMap() {
* @internal
* @param {*} component Statically passed child of any type.
* @param {*} parentType component's parent's type.
* @return {boolean}
*/
function validateChildKeys(component, parentType) {
if (Array.isArray(component)) {
Expand All @@ -174,10 +176,24 @@ function validateChildKeys(component, parentType) {
} else if (ReactElement.isValidElement(component)) {
// This component was passed in a valid location.
component._store.validated = true;
} else if (component && typeof component === 'object') {
monitorUseOfObjectMap();
for (var name in component) {
validatePropertyKey(name, component[name], parentType);
} else if (component) {
var iteratorFn = getIteratorFn(component);
// Entry iterators provide implicit keys.
if (iteratorFn && iteratorFn !== component.entries) {
var iterator = iteratorFn.call(component);
var step;
while (!(step = iterator.next()).done) {
if (ReactElement.isValidElement(step.value)) {
validateExplicitKey(step.value, parentType);
}
}
} else if (typeof component === 'object') {
monitorUseOfObjectMap();
for (var key in component) {
if (component.hasOwnProperty(key)) {
validatePropertyKey(key, component[key], parentType);
}
}
}
}
}
Expand Down
93 changes: 92 additions & 1 deletion src/core/__tests__/ReactElement-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,101 @@ describe('ReactElement', function() {

expect(console.warn.argsForCall.length).toBe(1);
expect(console.warn.argsForCall[0][0]).toContain(
'Each child in an array should have a unique "key" prop'
'Each child in an array or iterator should have a unique "key" prop.'
);
});

it('warns for keys for iterables of elements in rest args', function() {
spyOn(console, 'warn');
var Component = React.createFactory(ComponentFactory);

var iterable = {
'@@iterator': function() {
var i = 0;
return {
next: function() {
var done = ++i > 2;
return { value: done ? undefined : Component(), done: done };
}
};
}
};

Component(null, iterable);

expect(console.warn.argsForCall.length).toBe(1);
expect(console.warn.argsForCall[0][0]).toContain(
'Each child in an array or iterator should have a unique "key" prop.'
);
});

it('does not warns for arrays of elements with keys', function() {
spyOn(console, 'warn');
var Component = React.createFactory(ComponentFactory);

Component(null, [ Component({key: '#1'}), Component({key: '#2'}) ]);

expect(console.warn.argsForCall.length).toBe(0);
});

it('does not warns for iterable elements with keys', function() {
spyOn(console, 'warn');
var Component = React.createFactory(ComponentFactory);

var iterable = {
'@@iterator': function() {
var i = 0;
return {
next: function() {
var done = ++i > 2;
return {
value: done ? undefined : Component({key: '#' + i}),
done: done
};
}
};
}
};

Component(null, iterable);

expect(console.warn.argsForCall.length).toBe(0);
});

it('warns for numeric keys on objects in rest args', function() {
spyOn(console, 'warn');
var Component = React.createFactory(ComponentFactory);

Component(null, { 1: Component(), 2: Component() });

expect(console.warn.argsForCall.length).toBe(1);
expect(console.warn.argsForCall[0][0]).toContain(
'Child objects should have non-numeric keys so ordering is preserved.'
);
});

it('does not warn for numeric keys in entry iterables in rest args', function() {
spyOn(console, 'warn');
var Component = React.createFactory(ComponentFactory);

var iterable = {
'@@iterator': function() {
var i = 0;
return {
next: function() {
var done = ++i > 2;
return { value: done ? undefined : [i, Component()], done: done };
}
};
}
};
iterable.entries = iterable['@@iterator'];

Component(null, iterable);

expect(console.warn.argsForCall.length).toBe(0);
});

it('does not warn when the element is directly in rest args', function() {
spyOn(console, 'warn');
var Component = React.createFactory(ComponentFactory);
Expand Down
162 changes: 161 additions & 1 deletion src/utils/__tests__/traverseAllChildren-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,64 @@ describe('traverseAllChildren', function() {
);
});

// Todo: test that nums/strings are converted to ReactComponents.
it('should traverse children of different kinds', function() {
var div = <div key="divNode" />;
var span = <span key="spanNode" />;
var a = <a key="aNode" />;

var traverseContext = [];
var traverseFn =
jasmine.createSpy().andCallFake(function(context, kid, key, index) {
context.push(true);
});

var instance = (
<div>
{div}
{[{span}]}
{{a: a}}
{'string'}
{1234}
{true}
{false}
{null}
{undefined}
</div>
);

traverseAllChildren(instance.props.children, traverseFn, traverseContext);

expect(traverseFn.calls.length).toBe(9);
expect(traverseContext.length).toEqual(9);

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

it('should be called for each child in nested structure', function() {
var zero = <div key="keyZero" />;
Expand Down Expand Up @@ -231,4 +288,107 @@ describe('traverseAllChildren', function() {
);
});

it('should be called for each child in an iterable', function() {
var threeDivIterable = {
'@@iterator': function() {
var i = 0;
return {
next: function() {
if (i++ < 3) {
return { value: <div key={'#'+i} />, done: false };
} else {
return { value: undefined, done: true };
}
}
};
}
};

var traverseContext = [];
var traverseFn =
jasmine.createSpy().andCallFake(function(context, kid, key, index) {
context.push(kid);
});

var instance = (
<div>
{threeDivIterable}
</div>
);

traverseAllChildren(instance.props.children, traverseFn, traverseContext);
expect(traverseFn.calls.length).toBe(3);

expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
traverseContext[0],
'.$#1',
0
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
traverseContext[1],
'.$#2',
1
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
traverseContext[2],
'.$#3',
2
);
});

it('should use keys from entry iterables', function() {
var threeDivEntryIterable = {
'@@iterator': function() {
var i = 0;
return {
next: function() {
if (i++ < 3) {
return { value: ['#'+i, <div />], done: false };
} else {
return { value: undefined, done: true };
}
}
};
}
};
threeDivEntryIterable.entries = threeDivEntryIterable['@@iterator'];

var traverseContext = [];
var traverseFn =
jasmine.createSpy().andCallFake(function(context, kid, key, index) {
context.push(kid);
});

var instance = (
<div>
{threeDivEntryIterable}
</div>
);

traverseAllChildren(instance.props.children, traverseFn, traverseContext);
expect(traverseFn.calls.length).toBe(3);

expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
traverseContext[0],
'.$#1:0',
0
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
traverseContext[1],
'.$#2:0',
1
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
traverseContext[2],
'.$#3:0',
2
);
});

});
43 changes: 43 additions & 0 deletions src/utils/getIteratorFn.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/**
* Copyright 2013-2014, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule getIteratorFn
* @typechecks static-only
*/

"use strict";

/* global Symbol */
var ITERATOR_SYMBOL = typeof Symbol === 'function' && Symbol.iterator;
var FAUX_ITERATOR_SYMBOL = '@@iterator'; // Before Symbol spec.

/**
* Returns the iterator method function contained on the iterable object.
*
* Be sure to invoke the function with the iterable as context:
*
* var iteratorFn = getIteratorFn(myIterable);
* if (iteratorFn) {
* var iterator = iteratorFn.call(myIterable);
* ...
* }
*
* @param {?object} maybeIterable
* @return {?function}
*/
function getIteratorFn(maybeIterable) {
var iteratorFn = maybeIterable && (
(ITERATOR_SYMBOL && maybeIterable[ITERATOR_SYMBOL]) ||
maybeIterable[FAUX_ITERATOR_SYMBOL]
);
if (typeof iteratorFn === 'function') {
return iteratorFn;
}
}

module.exports = getIteratorFn;
Loading

0 comments on commit d545238

Please sign in to comment.