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

Children.map & Co do not traverse keyed Fragments #13677

Closed
arty-name opened this issue Sep 18, 2018 · 9 comments
Closed

Children.map & Co do not traverse keyed Fragments #13677

arty-name opened this issue Sep 18, 2018 · 9 comments

Comments

@arty-name
Copy link

Do you want to request a feature or report a bug?

Report a bug

What is the current behavior?

The documentation on React.Children.map tells us that "If children is a keyed fragment or array it will be traversed: the function will never be passed the container objects." This demo shows that arrays are indeed traversed, but the keyed fragments are not traversed. Open the developer console and click "Run" to see the output.

What is the expected behavior?

Children.map, forEach, toArray, count should traverse keyed Fragments as per documentation.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

The example runs on v16.5.1, but I have also seen that fail locally on v16.4.2. I never tried this before so I don’t know if this ever worked. I see it fail in latest Firefox and Chrome on Ubuntu Linux.

@arty-name
Copy link
Author

arty-name commented Sep 18, 2018

I think that the test I have added in #13679 also demonstrates the bug by failing

@jquense
Copy link
Contributor

jquense commented Sep 18, 2018

I don't think this is a bug actually, it's been discussed a few times in other issues and I believe the consensus was that it's intended that fragments be distinct elements vs an array of elements

@arty-name
Copy link
Author

@jquense so is this a bug in the documentation?

@jquense
Copy link
Contributor

jquense commented Sep 18, 2018

Sounds like it but let's wait for a core team member to confirm

@arty-name
Copy link
Author

Some way to automatically “flatten” the Fragments among the children will be a much desired feature anyway. I would be happy to see the code to work as currently specified in the documentation.

@bvaughn
Copy link
Contributor

bvaughn commented Sep 19, 2018

I don't think this is a bug actually, it's been discussed a few times in other issues and I believe the consensus was that it's intended that fragments be distinct elements vs an array of elements

Yes, it is intentional that .map does not traverse into Fragments. I'll update the docs. Apologies for the confusion!

@bvaughn bvaughn closed this as completed Sep 19, 2018
@arty-name
Copy link
Author

After more searching I have found this comment by Dan Abramov:

I think changing behavior for both might be reasonable.

Which I interpret as this implementation not being particularly intentional. He then proposes to submit an RFC for these changes, but I couldn’t find any at the moment. I will try to create one now.

@arty-name
Copy link
Author

RFC to flatten Fragments

@bvaughn
Copy link
Contributor

bvaughn commented Sep 20, 2018

Which I interpret as this implementation not being particularly intentional.

Maybe, although for what it's worth I talked to him to confirm this was intentional earlier yesterday:

Brian: Should React.Children.map treat Fragment as a single child? Docs are worded a bit ambiguously
Dan: Yeah that's intentional.
Brian: Cool, just wanted to confirm. I'll update the docs then.

That being said, an RFC sounds like the right way to go 👍

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

No branches or pull requests

3 participants