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.

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

Allow iterables in traverseAllChildren #2376

Merged
merged 1 commit into from
Oct 23, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 @@ -169,10 +169,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