You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add fragment API to allow returning multiple components from render #2127
We know this is an issue and we know exactly what set of problem can be solved. We want this too but it is a hard problem with our current architecture. Additional comments expressing desire for this feature are not helpful. Feel free to subscribe to the issue (there's button in the right hand column) but do not comment unless you are adding value to the discussion. "Me too" and "+1" are not valuable, nor are use cases that have already been written in the comments (e.g., we know that you can't put <tr> or <dd> elements with a <div>).
If you remove the <div></div> in the map, you get the following error: Adjacent XJS elements must be wrapped in an enclosing tag
it isn't till I re-add the surrounding, and rather pointless, divs that it compiles with out issue. I am running 0.11.1
Is this being addressed? It adds extra, and again - IMO - useless and pointless html to the page, that while harming nothing - looks messy and unprofessional. Maybe I am just doing something wrong, please enlighten me if I am.
I had a brief chat about it with chenglou a little while ago, I don't really lean one way or the other at the moment. I see a lot of hidden dangers in allowing a composite components to return multiple components and it breaks a lot of intuitive assumptions, but I don't know of any (right now) good practice use-cases that would obviously benefit from this.
The simplicity of returning at most one component means that it's very easy to reason about what you see, if not, <table><TableHeader /></table> could actually render any number of rows, you have no way of knowing other than inspecting TableHeader and whatever composite components it returns.
I feel like being able to return multiple components only moves the responsibility of wrapping components as necessary from the "composite component" to whatever "renders the composite component". The component "that renders composite component" rarely has or should have the knowledge of whether to wrap composite components or not, whereas children are more likely to have knowledge of their parents.
But perhaps it's simply a case of developer responsibility. There may be good use-cases for both and we should just look past the inevitable misuse.
However, even this doesn't work. Often, it's probably for the best. It can be confusing for consuming components when a child might expand into multiple elements.
But, really, the only reason we don't support this right now is because it's hard to implement. Hopefully, we'll be able to support it sometime in the future but probably not short term. Sorry. :/
can this not affect things like jquery or other libraries that target specific elements, so if you do something like $('#id-name').children(), the following:
Spent a couple of minutes understanding why my component didn’t work. I feel like there should be a notice somewhere, maybe I missed it ? Maybe it is obviously wrong to try :
@gabssnake You should have gotten a JSX compile error with the error "Adjacent XJS elements must be wrapped in an enclosing tag"; did you not see the error or was it not clear in its explanation?
Thank you for your answer @spicyj. Well, I meant a notice in the React documentation. Yes, the console did show an error, but the need to wrap didn’t make sense to me at first. That is why I searched and got here.
I've had this pain as well... particularly painful for my designer, as a matter of fact. It'd be kind of nice if a component could output a node (therefore node-list or fragment) instead of an element.
Reacted by shaharz, Jason Axelson, Elad Ossadon, Jay Phelps, snotr, Anton Mamrashev, Dmytro Lebedynskyi, Nicolas Escalante, William Stewart, Joseph Earl and 3 more
@gaearon You can do that already though, you just need to return an array for now (which is a bit cumbersome though yes) ... buuuuut, you can work around that, I have hacked my own <Frag> component which is translated to an array (overloaded React.render) ... you could also do return <NoopComp>...</NoopComp>.props.children I assume, if you want to avoid hacks.
EDIT: My bad, I overloaded React.createElement not React.render.
@gaearon Is there another issue for the <> syntax proposal that we can watch instead of further discussion on this issue? I searched around but I wasn't able to find one.
Reacted by Frédéric Miserey, Sergey Berezhnoy and Rikki Tooley
@smrq +1 to question — I follow everything about those awkward limitations (one-to-one rendrer() result, keys and JSX syntax or fragments) but the only ticket I know is react/jsx#65
I also suppose that fibers gonna fix issue with keys at all — but looks like it's unfulfilled dream
@spicyj actually it is an «issue» since it cause extended energy spendings from developers and there is fundamental possibility to program view framework without such requirement (for example https://github.com/dfilatov/vidom)
vidom does use keys in collections though. It might technically work without it but it would likely be much slower. React could also technically work without keys, but it'd be quite unexpected to find that half of your components have to update when you remove a single item from a list. With keys, only one item is unmounted.
@goto-bus-stop vidom can use keys, but they are not required and without them only really big cases with a lot of updates can cause real performance problems
so I consider this part as possible optional (as, for example, shouldComponentUpdate) which can be used for ferformance tweaks in individual cases
however, the unambiguous issue here (as @gaearon described) is a fully static usage of arrays and the difference between static arrays and static JSX fragments
@brigand O have no doubt that reordering of state-full components may cause some issues ;-) but force every other cases (in my opinion more of them) to struggle for this... looks controversial
Keys are important to React. Even if they don't seem to matter in some case (e.g. because components below are all stateless) there is nothing preventing you or someone else on the team to add a stateful component (or even a plain DOM input) somewhere a few levels below in a few months. By then you might forget that you don't have keys, and thus reorders may cause the state (or input value) to be associated with a wrong item.
This is why we encourage you to specify keys everywhere for dynamic lists. Not doing so leads to very hard-to-trace bugs, and we think it's better to spend extra ten seconds specifying the key than ten hours debugging why the state of a component several levels below gets messed up in some corner case.
I fully agree it's inconvenient to specify keys when the list is known to be static and never reorders. You are welcome to discuss this on the JSX repo. If you can't find an issue for this, you are welcome to create a new one there.
Since this thread has a lot of subscribers and the feature has been implemented I would like to lock it to avoid spamming a lot of people with notifications. I hope the clarifications above address your concerns, but if not, you are welcome to create a new issue for further discussion on a specific topic.
Activity
chenglou commentedon Sep 2, 2014
Because when you don't put the wrapper, it desugars to this:
Which doesn't make syntactical sense. The jsx compiler page might help if you need a visual mapping.
That being said, it's possible to desugar this to
[div, div]instead. This is hard, somewhat controversial and won't be implemented in a near future.sophiebits commentedon Sep 2, 2014
(I don't think it's particularly controversial but it does add code complexity and hasn't been done yet.)
chenglou commentedon Sep 2, 2014
IIRC @syranide has a few comments on this
syranide commentedon Sep 2, 2014
@chenglou Hehe.
I had a brief chat about it with chenglou a little while ago, I don't really lean one way or the other at the moment. I see a lot of hidden dangers in allowing a composite components to return multiple components and it breaks a lot of intuitive assumptions, but I don't know of any (right now) good practice use-cases that would obviously benefit from this.
The simplicity of returning at most one component means that it's very easy to reason about what you see, if not,
<table><TableHeader /></table>could actually render any number of rows, you have no way of knowing other than inspectingTableHeaderand whatever composite components it returns.I feel like being able to return multiple components only moves the responsibility of wrapping components as necessary from the "composite component" to whatever "renders the composite component". The component "that renders composite component" rarely has or should have the knowledge of whether to wrap composite components or not, whereas children are more likely to have knowledge of their parents.
But perhaps it's simply a case of developer responsibility. There may be good use-cases for both and we should just look past the inevitable misuse.
@sebmarkbage Probably has a few comments too :)
sebmarkbage commentedon Sep 2, 2014
We will probably never allow that syntax implicitly. You would need a wrapper like
OR
However, even this doesn't work. Often, it's probably for the best. It can be confusing for consuming components when a child might expand into multiple elements.
But, really, the only reason we don't support this right now is because it's hard to implement. Hopefully, we'll be able to support it sometime in the future but probably not short term. Sorry. :/
AdamKyle commentedon Sep 3, 2014
can this not affect things like jquery or other libraries that target specific elements, so if you do something like
$('#id-name').children(), the following:the
<div>and<div class="class-name">would be selected in this case. (If I understand this properly)DAddYE commentedon Oct 13, 2014
It affects also css selectors in the very same way @AdamKyle posted before.
srph commentedon Nov 16, 2014
Any updates on this issue?
gabssnake commentedon Dec 1, 2014
Spent a couple of minutes understanding why my component didn’t work. I feel like there should be a notice somewhere, maybe I missed it ? Maybe it is obviously wrong to try :
sophiebits commentedon Dec 1, 2014
@gabssnake You should have gotten a JSX compile error with the error "Adjacent XJS elements must be wrapped in an enclosing tag"; did you not see the error or was it not clear in its explanation?
gabssnake commentedon Dec 1, 2014
Thank you for your answer @spicyj. Well, I meant a notice in the React documentation. Yes, the console did show an error, but the need to wrap didn’t make sense to me at first. That is why I searched and got here.
GetContented commentedon Jan 28, 2015
I've had this pain as well... particularly painful for my designer, as a matter of fact. It'd be kind of nice if a component could output a node (therefore node-list or fragment) instead of an element.
gaearon commentedon Jan 28, 2015
Just saying.. I'm not advocating returning multiple children from component but I'd love to do that in
render*methods that I extract fromrender:But I should probably just shut up and use
keys.syranide commentedon Jan 28, 2015
@gaearon You can do that already though, you just need to return an array for now (which is a bit cumbersome though yes) ... buuuuut, you can work around that, I have hacked my own
<Frag>component which is translated to an array (overloadedReact.render) ... you could also doreturn <NoopComp>...</NoopComp>.props.childrenI assume, if you want to avoid hacks.EDIT: My bad, I overloaded
React.createElementnotReact.render.gaearon commentedon Jan 28, 2015
The problem with arrays is they trip our designer. Need commas, explicit keys.
128 remaining items
diligiant commentedon Aug 15, 2017
@JesperTreetop & @zwily, @gaearon explained it way better than I did ;)
Once you know, it's no big deal but as we all want React to thrive, I was just saying…
smrq commentedon Aug 15, 2017
@gaearon Is there another issue for the
<>syntax proposal that we can watch instead of further discussion on this issue? I searched around but I wasn't able to find one.veged commentedon Aug 15, 2017
@smrq +1 to question — I follow everything about those awkward limitations (one-to-one
rendrer()result, keys and JSX syntax or fragments) but the only ticket I know is react/jsx#65I also suppose that fibers gonna fix issue with keys at all — but looks like it's unfulfilled dream
sophiebits commentedon Aug 15, 2017
Keys aren't an "issue". :) They're a fundamental and necessary part of any view framework that allows you to create dynamic lists.
See https://facebook.github.io/react/tutorial/tutorial.html#keys for a more detailed explanation.
veged commentedon Aug 15, 2017
@spicyj actually it is an «issue» since it cause extended energy spendings from developers and there is fundamental possibility to program view framework without such requirement (for example https://github.com/dfilatov/vidom)
goto-bus-stop commentedon Aug 15, 2017
vidom does use keys in collections though. It might technically work without it but it would likely be much slower. React could also technically work without keys, but it'd be quite unexpected to find that half of your components have to update when you remove a single item from a list. With keys, only one item is unmounted.
veged commentedon Aug 15, 2017
@goto-bus-stop vidom can use keys, but they are not required and without them only really big cases with a lot of updates can cause real performance problems
so I consider this part as possible optional (as, for example,
shouldComponentUpdate) which can be used for ferformance tweaks in individual casesbrigand commentedon Aug 15, 2017
@veged example of vidom struggling without keys.
It has no idea it's supposed to reorder the elements, so it discards the instances on each render of the root component.
leeoniya commentedon Aug 15, 2017
as someone who's quite familiar with the virtual-dom space [1]. i can say that:
[1] https://github.com/leeoniya/domvm
veged commentedon Aug 15, 2017
however, the unambiguous issue here (as @gaearon described) is a fully static usage of arrays and the difference between static arrays and static JSX fragments
veged commentedon Aug 15, 2017
@brigand O have no doubt that reordering of state-full components may cause some issues ;-) but force every other cases (in my opinion more of them) to struggle for this... looks controversial
gaearon commentedon Aug 15, 2017
Keys are important to React. Even if they don't seem to matter in some case (e.g. because components below are all stateless) there is nothing preventing you or someone else on the team to add a stateful component (or even a plain DOM input) somewhere a few levels below in a few months. By then you might forget that you don't have keys, and thus reorders may cause the state (or input value) to be associated with a wrong item.
This is why we encourage you to specify keys everywhere for dynamic lists. Not doing so leads to very hard-to-trace bugs, and we think it's better to spend extra ten seconds specifying the key than ten hours debugging why the state of a component several levels below gets messed up in some corner case.
I fully agree it's inconvenient to specify keys when the list is known to be static and never reorders. You are welcome to discuss this on the JSX repo. If you can't find an issue for this, you are welcome to create a new one there.
Since this thread has a lot of subscribers and the feature has been implemented I would like to lock it to avoid spamming a lot of people with notifications. I hope the clarifications above address your concerns, but if not, you are welcome to create a new issue for further discussion on a specific topic.
sophiebits commentedon Aug 15, 2017
@smrq created a proposal issue for
<>syntax on the jsx repo: react/jsx#84.gaearon commentedon Nov 28, 2017
We just released support for a new
React.Fragmentexport and the associated<>syntax:https://reactjs.org/blog/2017/11/28/react-v16.2.0-fragment-support.html