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 to pass node prop type check #4209

Merged
merged 2 commits into from
Jul 6, 2015
Merged

Allow iterables to pass node prop type check #4209

merged 2 commits into from
Jul 6, 2015

Conversation

aruberto
Copy link
Contributor

#2376 add support for iterables as children. However if a component adds React.PropTypes.node validation to children, validation fails. This pull request adds an iterable check to ReactPropTypes.isNode()

@jimfb
Copy link
Contributor

jimfb commented Jun 24, 2015

lgtm, cc @spicyj


var iteratorFn = getIteratorFn(propValue);
if (iteratorFn) {
if (iteratorFn !== propValue.entries) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to check the values when iteratorFn === propValue.entries.

@camspiers
Copy link
Contributor

I don't completely understand what is going on here, but will this pull values out of generators, thereby meaning they won't have any next values when they are used within the component? Also, how does this deal with infinite iterators?

check that the value is a node for map iterables
@aruberto
Copy link
Contributor Author

Hey @spicyj, I've added scenario for when iteratorFn === propValue.entries

Now checks that value in each key-value pair of an entry iterable is a node.

@jimfb
Copy link
Contributor

jimfb commented Jun 25, 2015

ping @spicyj

@jimfb
Copy link
Contributor

jimfb commented Jul 6, 2015

Looks good to me, @spicyj's comments have been addressed, no complaints from anyone in almost two weeks, so let's ship it.

jimfb added a commit that referenced this pull request Jul 6, 2015
Allow iterables to pass node prop type check
@jimfb jimfb merged commit 500d4c3 into facebook:master Jul 6, 2015
@aruberto aruberto deleted the iterable_node_proptype branch July 9, 2015 15:37
@ksmithbaylor
Copy link

@camspiers Did you ever get your question addressed? This same scenario came up in #7536, and in doing some research I stumbled upon this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants