Add fragment API to allow returning multiple components from render
AdamKyle opened this issue ยท 148 comments
Note from maintainers:
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>).
Consider the following:
var ManagePost = React.createClass({
render: function() {
var posts = this.props.posts
var something;
var somethingelse;
var row = posts.map(function(post){
return(
<div>
<div className="col-md-8">
</div>
<div className="cold-md-4">
</div>
</div>
)
});
return (
{row}
);
}
});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.
Because when you don't put the wrapper, it desugars to this:
return React.DOM.div(...)React.DOM.div(...)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.
(I don't think it's particularly controversial but it does add code complexity and hasn't been done yet.)
@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 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.
@sebmarkbage Probably has a few comments too :)
We will probably never allow that syntax implicitly. You would need a wrapper like
<>
<div className="col-md-8">
</div>
<div className="cold-md-4">
</div>
</>OR
[
<div className="col-md-8">
</div>,
<div className="cold-md-4">
</div>
]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:
<div id="id-name">
<div>
<div class="class-name">
</div>
</div>
</div>the <div> and <div class="class-name"> would be selected in this case. (If I understand this properly)
Any updates on this issue?
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 :
var Optimistic = React.createClass({
render: function() {
return (
<h1>{this.props.name} loves React</h1>
<p>React doesnโt. Idea: sprinkle some divs here and there.</p>
);
}
});
React.render(
<Optimistic name="Peter" />,
document.getElementById('myContainer')
);@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.
Just saying.. I'm not advocating returning multiple children from component but I'd love to do that in render* methods that I extract from render:
render: function () {
return (
<div className={this.getClassName()}
style={{
color: this.props.color,
backgroundColor: this.props.backgroundColor
}}>
{condition ?
this.renderSomething() :
this.renderOtherThing()
}
</div>
);
},
renderSomething() {
return (
<>
<div className='AboutSection-header'>
<h1>{this.props.title}</h1>
{this.props.subtitle &&
<h4>{this.props.subtitle}</h4>
}
</div>,
{hasChildren &&
<div className='AboutSection-extra'>
{this.props.children}
</div>
}
</>
);
}But I should probably just shut up and use keys.
@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.
The problem with arrays is they trip our designer. Need commas, explicit keys.
@gaearon Yeah you can avoid the commas using either of my two workarounds for now (if you find either acceptable)... but what you do mean with explicit keys?
If I use array syntax, I need to specify key on each element. Not that it's hard to do, but feels awkward because I know they never change.
@gaearon Ah yes, I choose to just mentally ignore that warning for now :), if you really want to to avoid it, you can do <MyComp children={this.renderWhatever()} /> to avoid it (EDIT: although you obviously can't use that if you have adjacent children, you could use some flattening helper... but yeah).
Another case I've ran into with a UI kit. When you are placing children inside a fixed scrollable container like so:
return (
<div style={{
position: fixed
top: 0;
left: 0;
right: 0;
bottom: 0;
overflow-y: scroll;
}}>
{this.props.children}
</div>
);
The children now must be passed as an array, but if a composite class is passed, it must also implement those styles to avoid breaking it. Just adds a layer of complexity. But I can completely understand how complex it must be to make the change. Just throwing my hat in for support.
I have another use-case I described in detail over here #3415. But I can workaround it for now and understand that it is hard to implement and rare.
I know nothing about react internals (yet), but I'll just throw in an idea how you could mark such virtual parent elements/fragments in the DOM: comments. E.g.
<A>
<B></B>
<Fragment>
<C></C>
<D></D>
</Fragment>
<E></E>
</A>would render to
<a>
<b></b>
<!--<fragment data-reactid="">-->
<c></c>
<d></d>
<!--</fragment>-->
<e></e>
</a>This means c and d will be treated as children of sth. (including the nested reactid to not conflict with b and e). I've seen other frameworks "misuse" comments for these kind of semantical jobs.
@Prinzhorn I think you might be confusing DOM output with React's JSX.
In case it's useful: @Prinzhorn's suggestion of using HTML comments to map "fragment components" to the DOM is the same approach that Knockout uses. Knockout calls these "virtual elements".
<!-- ko component: "message-editor" -->
<!-- /ko -->
(from knockout docs)
Also, another use case for this - the extra wrapping elements can be problematic when using flexbox.
@aldendaniels absolutely agree, I've ran into flexbox issues already.
I ran into this with flexbox with dependent dropdowns. Where A would render and manage the first dropdown, followed by B or C or D depending on the value of A's dropdown, which would each render the appropriate number of dropdowns, e.g.
<A>
<Drop />
<C><Drop /><Drop /></C>
</A>
A, B, C, and D are stateless so I changed them from class B {render(){ this.props }} to function B(props){ return [...]; }.
In this case, it doesn't much matter to the parent that multiple children are rendered, it's just to deal with my CSS. There are things I'd do differently if these didn't need to be reused (another component needs B, C, and D).
As an alternative, maybe a way to unravel it from the parent? I don't have any specific ideas of how that'd look.
I hit a scenario today that I think is a good use case for this feature: A component that renders multiple <script> elements, where it could be rendering into the <head> element of the page. Any wrapper element there would be bad.
My scenario is wanting to have a component that is responsible for rendering the <script> tag both for the runtime code needed on the page as well as another <script> tag that carries the localized strings to be used by the runtime code. For example:
<html>
<head>
<script language="runtime.resources.en-us.js"></script>
<script language="runtime.js"></script>
</head>
<body>
...
</body>
</html>In this case, I'd like to have the code authored as:
var RuntimeScripts = require('./Runtime')
...
return (
<html>
<head>
<RuntimeScripts language="en-us" />
</head>
</html>
)
...I also ran into some flexbox issues. Nothing that can't be solved in CSS, but one of the "beauties" of flexbox is that you need less "wrapper" elements everywhere to make your layout work, but you'll still end up with wrapper elements everywhere when using React since you always wrap whatever you return in div/div or similar, unless it makes sense to have a container.
For all the use-cases presented here I'm pretty sure you could replace <BunchOfComponents /> with {getBunchOfComponents()} and the visual output would be the same, without introducing the practical and technical issues related to having components with fragments as root.
@syranide but every time one of the components changes all of its siblings need recalculating...
Also if you use plain coffeescript it's easy to return an array, so please decouple the functionality from the jsx representation.
IOW if it's easy to handle a returned array of elements, don't wait for jsx to catch up.
@syranide but every time one of the components changes all of its siblings need recalculating...
@wmertens Yes, but many times you would have that anyway because the parent would need to rerender for other reasons, or simply because you receive the data through props anyway. But yes, that is the difference, but it doesn't mean this approach is right, it's an optimization and there are many ways to accomplish them.
Also if you use plain coffeescript it's easy to return an array, so please decouple the functionality from the jsx representation.
That is irrelevant and this is not a problem with JSX. A big issue is that you lose the technical, practical and intuitive assumption of one component = one element/node. I can't speak for the devs but I would not give up that willingly, it's a very useful assumption to have. I'm sure there are equally good or better optimizations that could be designed if optimization is the only the reason people want this.
@syranide the biggest problem is that you can't always use a wrapping
element in html, like in tables, lists, flexbox, head... Working around
that leads to ugly code.
I would be perfectly happy with a virtual wrapper element that only renders
comments, as suggested before.
On Fri, May 29, 2015, 3:56 PM Andreas Svensson notifications@github.com
wrote:
@syranide https://github.com/syranide but every time one of the
components changes all of its siblings need recalculating...@wmertens https://github.com/wmertens Yes, but many times you would
have that anyway because the parent would need to rerender for other
reasons, or simply because you receive the data through props anyway. But
yes, that is the difference, but it doesn't mean this approach is right,
it's an optimization and there are many ways to accomplish them.Also if you use plain coffeescript it's easy to return an array, so please
decouple the functionality from the jsx representation.That is irrelevant and this is not a problem with JSX. A big issue is that
you lose the technical, practical and intuitive assumption of one
component = one element/node. I can't speak for the devs but I would not
give up that willingly, it's a very useful assumption to have. I'm sure
there are equally good or better optimizations that could be design if
optimization is the only the reason people want this.โ
Reply to this email directly or view it on GitHub
#2127 (comment).
fwiw, It's relatively easy to hack a "fragment" component into React that is treated as an array of its children by React. It will auto-generate keys, but since it happens after the initial validation of components, it won't throw the usual "no keys supplied" error.
With that said, that hack only solves what @gaearon was talking about above - not having to deal with ugly array syntax/setting arbitrary keys in your JSX - and not the issue of returning multiple nodes at the root of a component's render method.
I have a problem with the idea that a component needs to return one "element/node". To me, it seems perfectly reasonable for a JSX structure of:
<Main>
<Foo />
<Fragment>
<Bar />
<Baz />
</Fragment>
</Main>
to end up as the DOM:
<div>
<div>Foo</div>
<div>Bar</div>
<div>Baz</div>
</div>
I don't think this is a principle-of-least-surprise violation because components already do all kinds of "surprising things" with the DOM using lifecycle hooks (just look at the common wormhole pattern). It is generally accepted that a component will not necessarily result in a single element being created, and that's fine, because some compromises have to be made to work with the DOM.
This isn't about "optimizations," either, or even just about not liking array syntax. As many users have mentioned, wrapper elements break styling and layout in serious ways. Tables are the most obvious, but Flexbox is also a major issue. I already have CSS that just reapplies flex rules to wrapper elements that only exist because of React, and it's pretty ugly.
For all the use-cases presented here I'm pretty sure you could replace with {getBunchOfComponents()} and the visual output would be the same, without introducing the practical and technical issues related to having components with fragments as root.
This requires developers to compromise on making isolated, reusable components - heaven help them if they decide they want to reuse their bunch of components elsewhere - because of an underlying implementation issue in React. I don't think that should be accepted.
EDIT: My mistake, I conflated some of your arguments with the table-discussion above, I largely agree with what you're saying I think. But there are still problems with components being opaque, so what's intended as a useful transparent wrapper becomes opaque to the parent. Imagine <Wrapper1><Wrapper2>...</Wrapper2></Wrapper1>, Wrapper1 cannot see the children of Wrapper2. So perhaps wrapMyElements(...) simply is a better solution all-around still (including any other necessary supporting functionality).
I have a problem with the idea that a component needs to return one "element/node". To me, it seems perfectly reasonable for a JSX structure of:
Components are more than just dumb wrappers, they have a purpose. IMHO it seems that returning multiple elements blocks some very useful expectations. For example, React.render will get a companion in the future which renders an element and returns the nodes, this must now produce an array of nodes instead.
But I think a very important issue is that of readability which IMHO is the biggest selling point of React, everything is explicit.
<table>
<tr>
<td />
<td />
<td />
</tr>
<tr>
<Columns1 />
<Columns2 />
</tr>
</table>Looking at that it makes no sense, where is the 3rd cell coming from? Perhaps it's actually wrong and it's rendering 2 or 4 cells, who knows, perhaps it's actually dynamic and depends on a prop or external state? There are many of variations of this problem which only get harier when you consider other non-HTMLDOM frontends which may have explicit expectations. Another thing to consider is that elements are opaque, so if you replace <tr /> with <MyMagicalTr /> then it is not able to interact with the individual cells or even deduce how many there are, so even though <MyMagicalTr /> may only accepts <MyMagicalTd />'s there is no guarantee it can actually interact with them.
This requires developers to compromise on making isolated, reusable components - heaven help them if they decide they want to reuse their bunch of components elsewhere - because of an underlying implementation issue in React. I don't think that should be accepted.
"This requires developers to compromise on making isolated...", but that is exactly the problem if you ask me, if a component can return multiple elements it's no longer isolated, it's substituted, the component is leaking into the parent.
Hammer, nails. It being an underlying implementation issue is a separate issue from whether or not this should actually be done at all. It's not my decision, but I don't see how one rare use-case is convincing argument without considering the trade-offs that comes with it or what other alternative solutions there are.
IMHO I don't see the problem with {getBunchOfComponents()}, it's explicit, it allows us to keep our useful expectations. If performance is a problem then React.createSmartFragment() (or w/e) to the rescue, a transparent array/object-like type but one which can update independently from its parent.
Again, the React devs are the authority (not me), but I don't see a convincing argument here considering the various side-effects. I'm not even sure I agree with the presented solution being a good pattern at all, even if it was supported.
EDIT: To clarify, perhaps components may be able to return multiple elements in the future because there are other obviously beneficial use-cases, especially in the context of passing through children (like the one you show @thomasboyt), readability is maintained.
I think I'll need a bit more coffee before I can respond to the philosophical side of this conversation (thanks for the very good points, @syranide), but on the implementation side, I started poking around at this last night to see how viable a change of this scope is, leading to this spike: master...thomasboyt:fragment
And threw up a little demo here: http://www.thomasboyt.com/react-fragment-demo/
Some observations on the implementation side of things:
-
Not surprisingly, it's very tricky to retrofit a system that expects "1 component = 1 node" to support more nodes ;)
-
I initially thought to try tracking fragments on the DOM operations side, so that
ReactMultiChild's generated mutation instructions could stay the same and treat fragments like any other node. I couldn't think of a good way to add state about node counts/which nodes are fragments into the DOM state tracking, though. Something like the comment fencing @Prinzhorn noted could work, but I'm wary of anything that would require a DOM lookup, considering the relative cost. -
With that idea discarded, I added a
_nodeCountfield to all children of aReactMultiChildcomponent, so that it could track the number of root nodes a fragment actually contains.Problem is, while this is easy enough to do on an initial render by just counting the children of a fragment, updating the fragment's node count on subsequent mutations seems trickier. This is still not done on my branch (see thomasboyt#2).
-
Many DOM operations rely on accessing the parent node of an element, looked up by the internal node ID, to append/move/remove elements (see thomasboyt#3). Since the
ReactMultiChild's updateComponent cycle is responsible for passing this ID along, it could be changed to do a lookup of the closest parent that has a DOM node, but that sounds expensive. Alternatively, it may be possible to have an internal registry of fragment keys to their "real node" keys.
I'm still not convinced that requiring fragments to maintain a count of their root nodes is the best way to do this (though it did at least get me to that demo), and this was all hacked together quite rapidly and quite late at night, so if anyone else has a suggestion for implementation, feel free to chime in :>
@thomasboyt IIRC the main implementation obstacle comes from React referencing child nodes by mountIndex, this does not work when one "node" can suddenly become any number of nodes and this may happen without invoking the parent and it may also happen several components deep (wrapping). If I'm not mistaken it's pretty trivial to have React support multiple root elements as long as the number never changes.
So I don't think it would be especially hard to just make it work in React, but a truly proper solution is more problematic and should probably involve dropping mountIndex.
@syranide Right; the solution I'm working on actually introduces a new nodeIndex that is supposed to be the "real offset" of a node (which reminds me that I need to go back and remove mountIndex, since I think it's now unused in my branch).
But, as you note, this is problematic if the number of root elements changes, as the nodeIndex of a component needs to be updated whenever a previous sibling component's node count changes. Still need to find a solution for that.
I've also run into flexbox issues. @syranide could you please elaborate a bit more on your "getBunchOfComponents" proposed solution? Being new to React it's hard to fully get the idea where to define this function / how to apply it.
function getBunchOfComponents(...) {
return [<ColumnA key="a" />, <ColumnB key="b" />];
}Hey,
I've not read the whole thread but here's a render optimization usecase that may require this feature:
If this feature is released, ReactCSSTransitionGroup won't need a wrapper node anymore right?
@slorber Yes, that's probably true.
Run into the need for this feature everyday.
If you have many small components (ie design very modularly), you end up having to wrap all kinds of things in divs that shouldn't be. I might be conflating, but I think this relates to this issue.
For <div>'s you can wrap them in a <div>, but for table rows <tr> elements, it's not so easy. You can wrap <tr> in <tbody>, but may not be desirable to have multiple layers of <tbody> wrapping multiple layers of <tr>'s.
The scenario I called out was trying to have a component that provides <link> and <script> elements without having to become the <head> renderer entirely.
I added a note to the top of this issue. Please read it before commenting. #2127 (comment)
Bump...I have a large need for it on the server side. It's very complicated to render full web pages (excluding doctype) without the ability to render fragments, because of the <head> section. I'm currently working around that via mixins and a little logic in the final render, but it would be much simpler if there was support for rendering multiple components.
@IMPinball you can attempt to write something similar to react-document-title based on react-side-effect to solve these problems. I was able to do the same for meta tags, headers, title and occasionally redirects
I'm hitting this problem as well, are there any workarounds for the time being? I couldn't get {getBunchOfComponents()} to work as suggested.
None other than the ones already mentioned.
@jonchay You could create a component that only renders its children.
function statelessWrapper(props) {
return props.children;
}and then to use it:
render() {
return (
<statelessWrapper>
{renderABunchOfComponents()}
</statelessWrapper>
);
}@whatknight That won't work except in cases that return renderABunchOfComponents(); already works.
render () {
let user = this.state.user
let profile = user.get('data')
let view = null
if (user.get('status') === 'fetched') {
view = (
<h1>{profile.get('login')}</h1>
<img src={profile.get('avatar_url')} />
<dl>
<dt>email</dt>
<dd>{profile.get('email')}</dd>
</dl>
)
} else if (user.get('status') === 'fetching') {
view = <h1>fetching</h1>
} else if (user.get('status') === 'error') {
view = <h1>{profile.message}</h1>
}
return (
<div className={className}>
{view}
</div>
)
}There should at least be a way to return multiple fragments while doing interpolation and "assembly". The above example is complaining about img and h1 being adiacent but they will end up being inside the main wrapper anyway. That's one wrapper element I wish I could get rid of.
@kilianc in this case, you can simply write
view = [
<h1 key={0}>{profile.get('login')}</h1>,
<img key={1} src={profile.get('avatar_url')} />,
<dl key={2}>
<dt>email</dt>
<dd>{profile.get('email')}</dd>
</dl>,
]the way you're using it, it won't make a difference if this issue gets resolved.
I need this feature for already-stated reasons, so tried implementing a <frag></frag> container at https://github.com/mwiencek/react/tree/frag-component
The implementation isn't really pretty, but if it works for people I can submit a PR and let the React devs tear it apart.
@mwiencek Looks like your implementation doesn't work if the number of children in a fragment changes in an update (_nestedChildCount is set only in mountComponent)? There's a little trickiness to make that all work well. Looks like you've got a good start though. I've actually been thinking about this again recently and I may have figured out a robust way to make this happen. I'll report back if I find success.
@spicyj yup, you're right, I'll need to look into that...
Super stoked that we might see a proper implementation soon, though. :) Feel free to copy tests from that branch if they're of any use at all.
@spicyj Isn't the way forward to use createFragment and have JSX transform into that? Or do we really want fragments to be elements?
To build and expand on the last comment of @syranide, there seems to be no need for an extra "Fragment API" if render allows for arrays as return value. JSX could transform multiple root elements into an array, which would also work for return values of any other function. So instead of introducing additional API surface, which requires documentation and learning, one of React's limitations could just be removed.
This would at least affect babel-plugin-transform-react-jsx (implementation) and also babel-plugin-syntax-jsx(removing parse error for adjacent root elements). While changing the former seems to be fairly safe, I don't know the scope / usage of the latter and the impact the proposed change would have on other projects.
That still doesn't cover the use case of a conditional with multiple elements. I don't consider "Use an array and manually add an arbitrary key={...} to each element" to be proper a long term solution.
yup, good point. Automatic key generation should be built in via the transform. Using the array index as key should suffice, since the items are not changing.
As for conditionals, this could also be built into the transform or alternatively you could use JSX-Control-Statements. Implemented it there in this way, hence the idea.
To handle updates correctly I figured the solution @spicyj thought of for #5753 might work for fragments too (wrapping the contents in something like <!-- react-frag: 1 --><!-- /react-frag: 1 -->). Yeah the comments are a bit ugly, but it's way more reliable than what I was trying to do with _nestedChildCount. That approach is now what's used at https://github.com/mwiencek/react/tree/frag-component
I haven't seen this mentioned in the thread thus-far, but I think solving this also improves composability. For instance, imagine you have a grid where you want the cells to fade in a certain order. Ideally, there'd be two components at play here: one to handle the layout and another to handle the animation. You'd have an API like this:
<GridLayout
columns = { 3 }
>
<FadeAnimator
springConfig = { springConfig }
>
{ ...cells }
</FadeAnimator>
</GridLayout>This would enable you switch to a different layout, or a different animation, without one having to know about the implementation details of the other. GridLayout would expect to receive a list of children. FadeAnimator would intercept that list, inject the appropriate styles and/or event listeners, and return the new list for GridLayout to consume. There's no reason for FadeAnimator to be concerned with laying out a grid, except that React elements can't return Arrays from render. Moreover, there's no simple way to replace the grid with, say, a masonry layout, because FadeAnimator is being required to act as a container for its children.
With the current limitations, I suppose you could do something like this:
<FadeAnimator
wrapper = {
<GridLayout
columns = { 3 }
/>
}
springConfig = { springConfig }
>
{ ...cells }
</FadeAnimator>
// FadeAnimator
render() {
return React.cloneElement(
props.wrapper,
null,
props.children
);
}but that makes the code less clear, more complex, and harder to compose.
Add fragment API to allow returning multiple components from render๏ผ
Add fragment API to allow returning multiple components from render๏ผ
Add fragment API to allow returning multiple components from render๏ผ
Add fragment API to allow returning multiple components from render๏ผ
Add fragment API to allow returning multiple components from render๏ผ
Add fragment API to allow returning multiple components from render๏ผ
Add fragment API to allow returning multiple components from render๏ผ
Add fragment API to allow returning multiple components from render๏ผ
@texttechne suggestion is better. Instead introducing additional API, react should handle multiple root elements in render.
Handling multiple root elements in render would be hard I think.
It means that there: https://github.com/facebook/react/blob/master/src/renderers/shared/reconciler/ReactCompositeComponent.js#L1089
You would have an array of elements rather than an element.
Due to this, as far as I understand it, you'd have now to instantiate multiple React elements: https://github.com/facebook/react/blob/master/src/renderers/shared/reconciler/ReactCompositeComponent.js#L471
Then, mount multiple instantiated React elements: https://github.com/facebook/react/blob/master/src/renderers/shared/reconciler/ReactCompositeComponent.js#L471
And reconcile all the markup produced in the good order.
I think it comes with drawbacks that might not want to get around the reconciliation process.
Do we want to have fragments as elements or fragments as sugar syntax around transformations?
I think that fragment as an element is alright, we'd just need to create a new type of internal node similar to text nodes or empty nodes, right? Though I don't know how we'd manage them.
For instance, how do you handle when one of the roots is unmounted? How do you handle update properly?
Or, how do you manage multiple roots inside of the DevTools? (obvious answer: fix the DevTools...)
I think that a fragment is a composite component. Where is the difference exactly?
If we end up to duplicate code in order to implement fragments, we'd better implement sugar syntax in order to keep the React internals "pristine" ?
I'm wondering, I have been playing around with React internals around the Subtree question (renderSubtreeIntoContainer) and I feel like that it is somewhat related. When you want to render into a new subtree, you will have to render a new root in fact. So, if we support multiple roots at a tree level, do we render new roots each time:
<p>Hi</p>
<p>There</p>
would results in two "render into a new root" call.
Rather than one call if we used a wrapper, right? What about performance? Simplicity? To be honest, my feeling is: we should not touch to React internals to handle this situation. Rather, can we pull out this feat with JSX? Can we improve JSX syntax?
(Disclaimer: I'm not totally used to React internals, and there might be some parts that I don't fully understand or that I didn't understand. My apologies for misunderstanding.)
Edit: fixing/clarifying things. Also, GitHub mysteriously styles emails oddly, so I had to reformat the code block... :-(
Hi, core Mithril contributor/committer here.
TL;DR: Fragments are extremely hard, even when the API and internals are
simple.
By the way, I know from experience it's very difficult to implement. This
has also been requested multiple times for Mithril, but declined because of
the sheer difficulty. Every attempt to implement it has failed with at
least a third of the test suite failing.
I'm still working out details on a vdom library I'm planning on writing,
and it will treat everything as a fragment, but this is something you have
to literally (re)write the rendering portion from scratch for. Like React,
it'll be decoupled from the DOM, but the API will significantly differ
conceptually for rendering.
Here's the catch with fragments: you have to manage them completely
internally or you don't properly diff them. Even
document.createContextualFragment is useless. Just as an example, let's
transform two trees, rendering omitted:
// Before
A {}
fragment {
B[class="foo"] {}
B[class="bar"] {}
}
C {}
D {}
// After
A {}
B[class="foo"] {}
fragment {
C {}
}
D {}
The correct transform for this should be to replace both B elements and the C element, leaving the rest untouched. Figuring that out is non-trivial, and you have to basically iterate the fragment's children while ignoring the fact they're in a fragment.
But when the fragment's gone, you have to handle the hook semantics, like
shouldComponentUpdate (I don't remember React's name for that hook). So
you still have to track the fragments independently. You diff their
contents as if they were part of their parent fragment, but you still have
to keep track of that fragment's position for the component's sake.
Or in other words, components are no longer intrinsically linked to their
DOM node. Instead, they're linked to the corresponding fragment. React, like most other vdom libraries and frameworks, intrinsically couple the component to its tree representation, even with the expected types. This is the easiest way to implement a diff algorithm that
handles components. When they are decoupled, you have to do separate
bookkeeping for both. You don't initialize components when you initialize
the node. It's now two completely separate processes. It's hard to do
initially, and even harder to add support for afterwards.
Thanks all for the words. We know this is hard and still have it on our to-do list. (Asking enthusiastically won't make it happen sooner @janryWang.)
@isiahmeadows I think GitHub ate part of your comment. The code block is broken, and I can't see passed the first instance of <D />.
Looks OK in e-mail though. Maybe you found a bug in GitHub?
Unfortunately GitHub's markdown handling behaves differently when a comment comes from an email. I edited the comment to remove a blank line and now it shows up.
I forwarded the original e-mail to support@github. Hopefully, they can fix the parser. ๐
@lhorie You use array syntax for fragment ?
do you have polyfill for DocumentFragment ?
And using a "pseudo" wrapper element, like a HTML comment is not an option? I thought that was the way text nodes where "solved"...
Thanks for fixing that comment @spicyj
To address the concerns @isiahmeadows brought up in his example: the new Mithril engine does not follow the semantics that @isiahmeadows suggested for several reasons:
- implementing those semantics would make diffing significantly more complex
- it would make it difficult to reason about keys and key-related bugs since it becomes possible for key spaces to bleed out from components and even into sibling and subchild components.
- it would make fragment lifecycles non-intuitive (e.g. in that example,
B.baris removed, but so is the fragment, and a new fragment is created to wrap C). This violates the general principle that lifecycles "cascade", meaning that you can no longer be sure that a child node is removed if a given parent node is removed. Like the previous point, this has the potential to cause leakages on the encapsulation capabilities of a component. - if, hypothetically, one does run into diff issues related to a diff engine not adhering to those semantics, the application-space solution is as trivial as wrapping a fragment around the offending node.
I'd be interested if the core team could expand on the note at the top re: why this is hard with the current architecture. Seeing that React and Mithril's rendering engine are fundamentally trying to solve the same issues, and that Mithril does now support fragments to a degree that I believe to be feasible and useful, maybe implementing it in React might be more feasible if different aspects of the semantics are assessed separately (and potentially rejected) like it was done with Mithril.
Note that I fixed my comment. I made a few mistakes, and GitHub doesn't do well styling email responses... ๐ฆ
@Primajin I've wondered that too, but I suspect they'd be passed as one element. It's important to make fragments composable (see my example above). However, there are probably times you'd want to treat them as one unit too.
Perhaps, React.Children.map should expand fragments. If you want to iterate over every child (including children of child fragments), use Children.map. If you want to treat fragments as an opaque box, work directly with props.children as an {array, element}.
@lhorie I haven't been as involved in the rewrite, though, so I'm not as familiar with its intricacies. I've been busy with the fact I have one final this week and three next week, plus I'm working with someone to set up an internship my college requires. I've also been focused on finishing Techtonic, which I almost have the CLI done (a test is broken that shouldn't be).
@isiahmeadows Just a friendly reminder to stay on topic. Feel free to use the mithril gitter room if you want to chat about other topics.
Perhaps, React.Children.map should expand fragments
Mithril stable does something similar internally (i.e. flattening sub-arrays), but I'm moving away from that model due to perf reasons (and also due to some historical headaches regarding vnode lists that mixed keyed and non-keyed nodes). Might be something to consider.
And using a "pseudo" wrapper element, like a HTML comment is not an option? I thought that was the way text nodes where "solved"...
We've been using https://github.com/mwiencek/react-packages in production for a few months. It uses this comment wrapper approach, so fragments can be unambiguously nested.
@mwiencek are the comment wrappers even necessary? I would not expect clever things from fragments, if you move an element out of a fragment into a sibling fragment or the root element, it can just be recreated.
So if you follow the vdom tree in order, you don't need comments, or do you?
Anyway, your solution looks like it's exactly what's needed to resolve this issue, at first glance. ๐
Not strictly, but they made the implementation simpler in this instance.
So essentially it is currently not possible to create proper description lists <dl> with React?
<dl>
<dt>Def 1</dt>
<dd>Some description</dd>
<dt>Def 2</dt>
<dd>Some other description</dd>
</dl>
@KaiStapel This issue is about returning multiple components (or elements I guess) from render(). As long as your render function only returns one root element / component it should work.
OK:
render() {
return (
<dl>
<dt>Def 1</dt>
<dd>Some description</dd>
<dt>Def 2</dt>
<dd>Some other description</dd>
</dl>
)
}Not OK:
render() {
return (
<h2>my list</h2>
<dl>
<dt>Def 1</dt>
<dd>Some description</dd>
<dt>Def 2</dt>
<dd>Some other description</dd>
</dl>
)
}@GGAlanSmithee Well hardcoded yes, but you can not do:
<dl>
loop here and print out dt/dd pairs
</dl>
Which is very sad. Same also goes for tables with rowspans, since you can not render two <tr> elements at once :(
From the top:
"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>).
Given that there is a working solution in https://github.com/mwiencek/react-packages, is there any chance this will be part of React proper soon? Or are we waiting on the new reconciler?
Given that there is a working solution in https://github.com/mwiencek/react-packages
Are you using it successfully in real projects?
The implementation isn't really pretty, but if it works for people I can submit a PR and let the React devs tear it apart.
Sure, please send a PR!
About submitting a PR, the last I heard from @spicyj was that they wanted to finish the new core algorithm and do a proper solution with that, since the comment nodes don't really make sense in React Native. I haven't been following the status of that, but I don't think those plans have changed. I'm glad people find the packages useful in the meantime.
The new algorithm is in the works and supports fragments. However I wouldnโt expect it to become production ready for months to come. I wonder if adding this first to React DOM and later to React Native is too bad? The downside is it fragments the ecosystem a little (pun intended!), but it may give us some time to experiment with that feature. We have a team meeting today so Iโll raise this question if we have time to get a better understanding.
@gaearon Can I just point out that supporting fragments is super simple, it's just sugar, I'm currently using <frag> myself by way of a tiny trivial wrapper. Is returning multiple children as the component root really that important?
@syranide I am using the custom build with frag in the beta environment, however I would like to use official React build instead. Can you provide your <frag> wrapper? :) Thanks
import React from 'react';
import createFragment from 'react-addons-create-fragment';
let nativeCreateElement = React.createElement;
React.createElement = function() {
if (arguments[0] !== 'frag') {
return nativeCreateElement.apply(this, arguments);
}
let length = arguments.length;
if (length <= 2) {
return null;
}
let children = {};
for (let i = 2; i < length; i++) {
children['~' + (i - 2)] = arguments[i];
}
return createFragment(children);
};We talked more about this in the last team meeting. The consensus is that we donโt want to go with this particular implementation. However this feature will be supported in the long term core rewrite (no timeline on this for now).
We will also consider it again as one of the things we could potentially work on for the second half of this year if the rewrite takes too long or doesnโt work out. No guarantees it will make the list but we will keep you all updated if it comes up.
To get a better idea of what we work on, please see our meeting notes repo! You can find our latest discussion on this in https://github.com/reactjs/core-notes/blob/master/2016-07/july-07.md.
@gaearon It would be interesting with traction on having an official frag-syntax at least.
@syranide Thanks for the code, but unfortunately it seems that I cannot use it as I need the frag as the root app component that is being renderer by ReactDOM.render method and this method will not accept fragment.
Thanks anyway, It will be useful for other people that don't need the frag as app root.