-
Notifications
You must be signed in to change notification settings - Fork 48.8k
Reimplement React.Children #9339
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
Reimplement React.Children #9339
Conversation
What do you think about completely rewriting Would you be interested? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment—maybe we can both avoid allocation and make the code less abstract.
@koba04 happy to work with you on that ^ if you'd like! |
Could we remove It's not used in other React sources. I'm not sure whether it's being used in outside React, but it is an undocumented API. |
I don't see it being used in |
We have an internal dependency on it at FB but we'll fix it in another way. So feel free to skip it. |
63c5ed2
to
5c79c79
Compare
8e151f8
to
d2626ad
Compare
forEachFunc: () => mixed, | ||
forEachContext?: Object, | ||
): void { | ||
mapChildren(children, forEachFunc, forEachContext); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to implement forEach
in terms of map
because we don't want to allocate the result just to ignore it. It's fine to duplicate the code in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I've fixed it!
1379048
to
771a532
Compare
90fc5e5
to
61981da
Compare
duplicate of #10227 |
Hey, sorry I didn't provide more feedback on this. As you see from my implementation I still wanted to keep the pooling. Thanks for working on it though, it helped me gather my thoughts and figure out how to go about this. |
No problem! 😄 |
@aweary
This is a fix for #9325 (comment)