React.cloneElement breaks children lifecycle events

[milesj]

I'll try to make this as easy to understand but will probably be pretty lengthy.

I'm in the process of converting my Toolkit library to React and started with the Accordion component. This Accordion his 4 components, the Accordion parent wrapper which handles state, events, etc, the AccordionItem which is nested within Accordion, and the AccordionHeader and AccordionSection which are rendered within AccordionItem. This can be seen here (very rough): https://github.com/titon/toolkit/blob/3.0/js-3.0/ui/components/Accordion.jsx

And can be implemented as such:

import Accordion from './ui/components/Accordion.jsx';

<Accordion>
    <Accordion.Item header="Header #1" key="1">
        <p>Phasellus viverra convallis ex sit amet convallis. Sed accumsan dignissim massa, eu volutpat tellus semper at. Quisque non lectus sit amet lectus consectetur tincidunt nec in sem. Fusce lobortis blandit turpis, vel vestibulum nulla egestas vitae. Vivamus quis orci vitae odio elementum facilisis. Vestibulum suscipit quam in dictum ullamcorper. Sed lectus quam, faucibus id pellentesque nec, suscipit at elit. </p>

        <p>Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Morbi fermentum congue lectus, at imperdiet odio dictum sit amet.</p>
    </Accordion.Item>
    <Accordion.Item header="Header #2" key="2">
        <p>Sed non sem tempor, feugiat risus at, dignissim ipsum. Ut efficitur lacus non sem laoreet tincidunt. Proin porttitor placerat massa, eu varius libero elementum ac. Donec a ligula ullamcorper, pulvinar leo vel, vestibulum magna. Aliquam et tempus tortor.</p>

        <ul>
            <li>Nulla leo nulla, porta in tortor in.</li>
            <li>Cras sed finibus felis.</li>
            <li>Nulla purus turpis, lobortis at viverra id.</li>
        </ul>
    </Accordion.Item>
</Accordion>

The issue I'm running into is: How do I pass props and information from the parent Accordion to the AccordionItem children, seeing as how those are implemented manually by a developer, and not generated by the parent component? I simply can't define explicit props.

The way I currently solved this is by cloning all the children and passing custom props, as seen here (and suggested all over Google): https://github.com/titon/toolkit/blob/3.0/js-3.0/ui/components/Accordion.jsx#L154

The problem with this approach is that the children are re-cloned every time the parent renders, which breaks all the child lifecycle events. Each lifecycle event, like componentWillReceiveProps(), will receive all the same props, every render, making it extremely difficult to see what exactly changed. On top of this, constantly re-cloning everything seems like an expensive process.

I also tried modifying the child's props through React.Children.map(), but they are all read-only of course, which is quite annoying in this instance.

I tried cloning the children only on the first render, and attempting to update single props on each subsequent render, but that turned out to be really difficult or nigh impossible.

I thought about using an event system from within the parent, but how can I hook up the children to listen to it? Still couldn't figure that out.

I'm now looking at contexts (https://facebook.github.io/react/docs/context.html), but they seem very new and unstable.

I feel like this is such a basic and common use case that it should be supported, yet it's a giant pain right now. I'm not the only one having this issue either, it's all over StackOverflow.

So what exactly is the best approach to solving this? Contexts? Deal with the cloning issue? Any advice would be appreciated.

[jimfb]

This is a usage question, rather than an issue in React. For this reason, I'm going to close it out. Usage discussions are better asked on StackOverflow, as we use github issues for tracking bugs in the React core.

This is a pattern known as "wormhole". There are currently a couple of workarounds (like cloneElement, context, or data-driven), but they are admittedly imperfect. We've been discussing the pattern for a long time, but we haven't yet come up with a "blessed" solution. When we do, we'll be sure to communicate our ideas/design.

Cloning the element is usually a fine solution, using context would also work. I'm not entirely sure what you mean by "breaks all the child lifecycle events". Having componentWillReceiveProps() get called multiple times is expected, regardless of whether or not you clone. Re-cloning really isn't very expensive.

The data-driven solution (which was one of the workarounds I mentioned above, but the one you didn't mention) is something like:

var myData = [new AccordionItemData("title", <p>body</p>), new AccordionItemData("other title", <p>other body</p>)];
<Accordion data={myData} />

Then you iterate over your data and render the accordion items with any data you want. If you want to allow people to customize their accordion item, you can take in a function (as a prop) that takes in the title+body+whateverYourAccordionWantsToProvide and returns JSX representing the new AccordionItem with appropriate props. That might look something like this:

var myData = [new AccordionItemData("title", <p>body</p>), new AccordionItemData("other title", <p>other body</p>)];
function MyCustomAccordionItemRenderer(title, body, isCollapsed, whatever) {
  return <div className={isCollapsed ? "invisible" : "visible"}><h1>{title}</h1><div>{body}</div>;
}
<Accordion data={myData} myAccordionItemRenderer={MyCustomAccordionItemRenderer} />

Anyway, good luck with your app! Closing this out as per above, but free to continue the discussion on this thread or move the discussion to StackOverflow.

[milesj]

Well the reason I reported it is because the values passed to componentWillReceiveProps() are always the same. For example, according to this: https://github.com/titon/toolkit/blob/3.0/js-3.0/ui/components/Accordion.jsx#L154 That lifecycle event would receive uid, index, currentIndex, and onClickHeader, instead of only the props that changed.

This makes it impossible to determine what the previous state was, since the cloned element is completely fresh. The same applies to will/did lifecycle events as the next and previous props/state are useless. I know this might not be fixable, as it's a side effect of cloning, but thought I would report it anyways.

I've also looked into that data-driven solution, but not sure I like it. It requires another layer of unnecessary complexity.

[jimfb]

This makes it impossible to determine what the previous state was, since the cloned element is completely fresh. The same applies to will/did lifecycle events as the next and previous props/state are useless. I know this might not be fixable, as it's a side effect of cloning, but thought I would report it anyways.

This statement confuses me. Cloning the element does not create a fresh instance.

[milesj]

The children are all re-cloned when the parent is rendered, hence a new cloned instance.

Anyways, I was able to solve most of my issues using contexts and requiring all children to explicitly pass their "index" prop. Not ideal, but works for now. Hopefully there's a better way to do this in the future. Done here: https://github.com/titon/toolkit/blob/3.0/js-3.0/ui/components/Accordion.jsx

On a related note, it would be nice if all children were passed an index of their position in the children list.