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

React.Children.toFlatArray #61

Closed
wants to merge 1 commit into from
Closed

React.Children.toFlatArray #61

wants to merge 1 commit into from

Conversation

arty-name
Copy link

@arty-name arty-name commented Sep 20, 2018

View formatted RFC

Per a suggestion from Dan Abramov: this RFC proposes adding a new API React.Children.toFlatArray to get the children as an array while treating the Fragments as transparent containers.

@TryingToImprove
Copy link

TryingToImprove commented Sep 20, 2018

I like the proposal, but I think that there should be a dept like with Array.flat, but seems like such a thing would allow developers to take the entire three in their Root element.

I would prefer a option as (as the # 1 alternative)

React.Children.toArray(children, { threatFragmentsChildrenAsChildren: true })

# Drawbacks

- larger API surface due to the new API
- the rest of the APIs in the `React.Children` namespace keep their potentially surprising behaviour

Choose a reason for hiding this comment

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

I would also be surprised by this, not only to sole fact that Fragments are not flattened - as this can be more or less easily memorised, but surprising to me is that regular methods flatten array (& iterators), but not Fragments. This is imho inconsistent slightly and will be even more surprising to see some children being already flattened by no-flattening (taking into account its name) method when you introduce API with the name flat in it.

Copy link
Author

Choose a reason for hiding this comment

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

Unfortunately I don’t know the motivation behind the current state of affairs but I assume that there are reasons for this intentional inconsistency. Hopefully the developers will explain them in the comments here.

@gaearon
Copy link
Member

gaearon commented Oct 19, 2018

In general we've been hesitant to touch React.Children. It's a leaky abstraction because generally it's surprising that replacing <Foo /> with a component rendering <Foo /> somehow changes the behavior. Same applies for this proposal, and how extracting <Fragment> inside a component would suddenly change it.

Instead, we need to figure out a better long term API to address use cases that are currently served by Children. React Call Return was one attempt but not particularly successful, and we removed it.

@arty-name
Copy link
Author

I think I have failed to explain my idea well enough. The current behaviour of React.Children with <Fragment> is already problematic and I would like to have an unproblematic way to handle them. Either by this new API or by changing existing behaviour of React.Children. So the result my proposal will not be changed by extracting a <Fragment>.

It would be nice to have a more convenient stop-gap solution until you guys figure out the right approach.

@birkir
Copy link

birkir commented Oct 28, 2018

Something like this perhaps? https://github.com/birkir/react-children-addons#toflatarray

Example: https://codesandbox.io/s/p33llrxz3q

@dreampulse
Copy link

Is there a solution on the horizon?

@grrowl
Copy link

grrowl commented Nov 20, 2019

I just open-sourced some code from my own projects which handles this (in a specific way which ensures stable keys like Children.toArray does), would love feedback if anyone uses it! https://github.com/grrowl/react-keyed-flatten-children

@gaearon
Copy link
Member

gaearon commented Feb 10, 2020

Since https://github.com/grrowl/react-keyed-flatten-children seems to implement this and React.Children is in maintenance mode (and will not be recommended after we figure out something like call/return), I'm going to close this.

@gaearon gaearon closed this Feb 10, 2020
adamkudrna added a commit to react-ui-org/react-ui that referenced this pull request May 30, 2020
Fragments are not traversed with `React.children.map()` [1][2] so we are using the `react-keyed-flatten-children` bundle to flatten the children to a one-dimensional array. This is necessary to be able to pass props to children of these layouts.

[1] https://reactjs.org/docs/react-api.html#reactchildrenmap
[2] reactjs/rfcs#61 (comment)
adamkudrna added a commit to react-ui-org/react-ui that referenced this pull request Jun 2, 2020
Fragments are not traversed with `React.children.map()` [1][2] so we are using the `react-keyed-flatten-children` bundle to flatten the children to a one-dimensional array. This is necessary to be able to pass props to children of these layouts.

[1] https://reactjs.org/docs/react-api.html#reactchildrenmap
[2] reactjs/rfcs#61 (comment)
adamkudrna added a commit to react-ui-org/react-ui that referenced this pull request Jun 5, 2020
Fragments are not traversed with `React.children.map()` [1][2] so we are using the `react-keyed-flatten-children` bundle to flatten the children to a one-dimensional array. This is necessary to be able to pass props to children of these layouts.

[1] https://reactjs.org/docs/react-api.html#reactchildrenmap
[2] reactjs/rfcs#61 (comment)
adamkudrna added a commit to react-ui-org/react-ui that referenced this pull request Jun 5, 2020
Fragments are not traversed with `React.children.map()` [1][2] so we are using the `react-keyed-flatten-children` bundle to flatten the children to a one-dimensional array. This is necessary to be able to pass props to children of these layouts.

[1] https://reactjs.org/docs/react-api.html#reactchildrenmap
[2] reactjs/rfcs#61 (comment)
nightflash added a commit to JetBrains/ring-ui that referenced this pull request Sep 16, 2024
nightflash added a commit to JetBrains/ring-ui that referenced this pull request Sep 16, 2024
andrey-skl added a commit to JetBrains/ring-ui that referenced this pull request Oct 21, 2024
* RG-2293 Get rid of side effects in setState updater function of Select component (#7574)

* 6.0.50

* RG-2462 switch sidebar and main background in dark theme (#7565)

(cherry picked from commit 647dd76)

* 6.0.51

* Support passing theme down via React Context (#7592)

* 6.0.52

* RG-2420 chore: undeprecate "Badge" in 6.x version

* Chore: export GLOBAL_DARK_CLASS_NAME constant [publish]

* 6.0.53

* JT-83359 Enable back controlling shortcuts by having '.ring-js-shortcuts' on parent, removed in 473d554 (#7637)

* RG-2473 don't unmount react root because it can be reused later (#7656)

minor: log auth error

* 6.0.54

* Revert "JT-83359 Enable back controlling shortcuts by having '.ring-js-shortcuts' on parent, removed in 473d554"

This reverts commit 9192646.

* 6.0.55

* Put page reload in timeout for USER_CHANGED event to let all other events to be called. JT-83975 related

* 6.0.56

* Chore: add example with dialog with close button inside

* RG-2227 Clicking on a select label reopens the dropdown

(cherry picked from commit a685a07)

* chore: Prevent dialog scroll (#7710)

* chore: Prevent dialog scroll

* chore: preventBodyScroll should not be passed as a part of restProps

* 6.0.57

* Add attributes prop to DatePicker button (#7727)

Co-authored-by: Mikhail Cherviakov <[email protected]>

* 6.0.58

* [publish] fix breadcrumbs work with React.Fragment containers as a transparent containers

reactjs/rfcs#61

* 6.0.59

* Revert "[publish] fix breadcrumbs work with React.Fragment containers as a transparent containers"

This reverts commit 704bb21.

* 6.0.60

* Fix Table colspan calculation when columns is passed as function

* 6.0.61

* Get rid of unused Table maxColSpan prop

* RG-2490 support moving cursor from anchor to tooltip (#7754)

* RG-2490 support moving cursor from anchor to tooltip

* RG-2490 use React for "mouseout" event

* 6.0.62

* Check className in QueryAssist shouldComponentUpdate

* 6.0.63

---------

Co-authored-by: Veniamin Krol <[email protected]>
Co-authored-by: JetBrains Ring UI Automation <[email protected]>
Co-authored-by: Filipp Riabchun <[email protected]>
Co-authored-by: Alexander Burkov <[email protected]>
Co-authored-by: Mihail Ch. <[email protected]>
Co-authored-by: Mikhail Cherviakov <[email protected]>
Co-authored-by: maksim.erekhinskii <[email protected]>
Co-authored-by: Veniamin Krol <[email protected]>
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.

8 participants