reactjs
Reparenting API
Terminology and naming for the createReparent function are up for discussion.
Does this RFC suggest that React would move around the actual DOM nodes? Is it technically possible? For example, if one of the child node was a playing <video> element, by removing it from DOM and reinserting into a different parent, would it still be playing?
@streamich Yes, DOM nodes would be detached with removeChild if the reparent tree is not rendered, and re-inserted later with appendChild/insertChild when added back.
If one of the children of the reparent is a video element this does imply that the video element inserted in the other parent would be the exact same video element.
I cannot make guarantees however about the video still playing. Even if you keep the same instance of the dom node, some DOM elements have quirks when you move them around the dom tree. I'm not proposing any special hacks to handle that. So the video will be the same, but if videos stop playing when you appendChild them into another parent then videos will stop playing when you reparent them.
Though on the topic of playing videos. Take a look at my other "registered prop as ref" RFC, one of my examples is a custom Playing implementation. Which would make the video element a controlled component in regards to playing (like how inputs are controlled when you pass a value to them). Theoretically if you reparented a video with a controlled Playing attribute then even if the browser stopped it the controlled nature would make it resume after it is reparented (though you might have to ignore a pause event if the browser fires one).
...if you reparented a video with a controlled
Playingattribute then even if the browser stopped it the controlled nature would make it resume after it is reparented...
I think it would reset the currentTime of the video, too.
I think it would reset the currentTime of the video, too.
Well. If you really wanted. You could build a <Video> component which would make all the state in a <video> tag controlled (playing, currentTime, etc...) in the normal way for react (register events to keep state up to date like firing setState every time the video time change, map things to props and change them when props change, etc...). Then this RFC would guarantee that if your <Video> was reparented the React component would stay intact with all its state and in componentDidUpdate you could ensure that any controlled state that the browser reset from the reparent would be restored by your Video component.
@dantman The whole point of reparenting is to keep intact all and any of the states of element that are not controlled by React.
Imagine yourself making something like Jupyter. You have a component that wraps CodeMirror, and you'd like to drag and drop rows of the document so that all the state of CM stays the same: selection, focus, scroll etc. There is no way to control all of this state by the component because CM doesn't provide API methods to control every single part of it. In the unlikely case that controlling all props works out with a <video> element, that's fine, but you need no reparenting for that. Reparenting exists exactly for the cases when you cannot do it.
@streamich Yes, it's as simple as appendChild on a component that is already appended somewhere else, without any removeChild calls.
I guess, for React just re-parenting and leaving it up to the user to deal with consequence is fine as long as it is opt-in. Here is a short list of consequences though:
<video>will stop and lose its state<audio>will stop and lose its state<iframe>will reload- Any focused elements will lose their focus?
- Very important one: any
.swfFlash players will reload 🤣 - Probably a bunch of other things will lose their state or reload.
Seems to have some overlap with #7? dangerouslyRecycleNodesounds like a similar concept to me, though I admit I haven't deeply read through either proposal.
@polkovnikov-ph
There are dom states that do not get discarded when you call removeChild on them. And the ability to retain React Components while reparenting is just as important.
But my intent with mentioning removeChild is for the situations where you intentionally detach a tree and hold on to a reference to later mount it.
class MyComponent extends Component {
content = React.createReparent(this);
render() {
const {show, children} = this.props;
const content = this.content(<Fragment>{children}</Fragment>);
if ( show ) {
// Show the contents
return content;
} else {
// Don't render the contents, the dom/component tree will not be
// present in the page but it will be retained as a detached tree
// and later re-used when `show` becomes true again.
return null;
}
}
}
If you move a dom node from one parent to another in a render such that the reparent is never left out of the react component tree, I do think it would be ideal for React to just use appendChild/insertChild to move the node instead of using removeChild and then inserting it in the new parent.
If there are dom states that get discarded when you removeChild->appendChild in the same tick but not when you just appendChild. I can add a note to the design/implementation section noting that the implementation should not use removeChild unless the tree is actually being detached and should only use appendChild/insertChild calls when it is only necessary to move dom nodes.
I can also add a limitations section noting the dom nodes that lose state when you reparent them with appendChild.
Would that be sufficient?
I'm unsure, but it seems that it's possible to create a loop of reparenting components that locally looks as if it's possible to use only appendChild on them, but it actually requires at least one removeChild to break the loop. I'm too tired today to convert this idea of counterexample into code tho :/
To dive more into this issue of removeChild/appendChild and state loss I've built a series of simple dom reparent tests that will reparent an element using just appendChild, removeChild + appendChild, and using a delay before re-insertion. Feel free to propose other elements
https://dantman.github.io/dom-reparent-tests/
Summary
- Chrome is the only browser that pauses video/audio when you reparent something with appendChild
- Firefox and Safari won't even pause the video when you removeChild+appendChild, until you put a delay between the calls, they'll just keep playing the video at the new position
- IE11 was the only browser that had any difference between appendChild and removeChild+appendChild – pausing the video when you removeChild+appendChild. Though given that Chrome always pauses, this is actually better than Chrome!
- Absolutely none of the browsers I tested lost the current position of the video, even when the video was detached from the tree and a delay was used before putting it back. The suggestion that
currentTimegets reset seems to be invalid. - iframes do reload in all browsers, no matter what. This is just a limitation that will have to be accepted for iframes as even just using appendChild without removeChild makes no difference.
- Likewise scroll positions are the same, they always reset and you cannot avoid it by just using appendChild. Please see RFC #33, it suggests a new API that solves the scroll issue for normal content updates and potentially could also be used to retain scroll state when reparenting. It may even work for retaining other states like focus and selection.
Conclusion
It would be nice to still note that React should try to use appendChild without first using removeChild if it does not need to. However this actually isn't a big deal. In practice it appears that almost nothing is lost during removeChild that is not lost during appendChild (as long as you re-insert within the same tick). The only difference I could find is that IE11 will pause the video when you removeChild+appendChild but not appendChild. But this is a pretty pointless difference because Chrome pauses the video on even a basic appendChild, so you are going to have to make the playing state a controlled behaviour anyways.
Results
| Chrome | append | remove+append | delay |
|---|---|---|---|
| video | pauses, but doesn't lose position | pauses, but doesn't lose position | pauses, but doesn't lose position |
| audio | pauses, but doesn't lose position | pauses, but doesn't lose position | pauses, but doesn't lose position |
| iframe | refreshes | refreshes | refreshes |
| scroll | loses scroll position | loses scroll position | loses scroll position |
| Firefox | append | remove+append | delay |
|---|---|---|---|
| video | does not pause or lose position | does not pause or lose position | pauses, but doesn't lose position |
| audio | does not pause or lose position | does not pause or lose position | pauses, but doesn't lose position |
| iframe | refreshes | refreshes | refreshes |
| scroll | loses scroll position | loses scroll position | loses scroll position |
| Safari | append | remove+append | delay |
|---|---|---|---|
| video | does not pause or lose position | does not pause or lose position | pauses, but doesn't lose position |
| audio | does not pause or lose position | does not pause or lose position | pauses, but doesn't lose position |
| iframe | refreshes | refreshes | refreshes |
| scroll | loses scroll position | loses scroll position | loses scroll position |
| IE11 | append | remove+append | delay |
|---|---|---|---|
| video | does not pause or lose position | pauses, but doesn't lose position | pauses, but doesn't lose position |
| audio | could not test with .wav | could not test with .wav | could not test with .wav |
| iframe | refreshes | refreshes | refreshes |
| scroll | loses scroll position | loses scroll position | loses scroll position |
Something else that is hard to test is selection behaviour. This probably isn't an issue for overall selection (I notice that even when completely removing and creating dom nodes a selection outside of them will update to contain even whole block nodes). However it may be relevant for reparenting large paragraphs with some selected text in them or large textarea inputs with a selection being edited.
Important discussion points:
Is it possible to hydrate a server-rendered DOM tree and know, without needing deterministic IDs, and know what portion of it belongs to a Reparent and should be detached and held on to when unmounting instead of destroyed? (Or would we need to wait for the internals of https://github.com/reactjs/rfcs/pull/32 to solve this)
Should the reparent function (this.header(children)) accept multiple children with a spread, the same way createElement accepts multiple children? These would simply be passed on to createElement, so the Fragment could contain more than just one child. Or is this API confusing and we should just recommend people needing this pass in a Fragment as the child of the Reparent?
When a Reparent's tree is detached from the DOM (the DOM nodes are not part of the document, but the tree still exists so it can be re-inserted later) and the Reparent is given new children. Should it render those children into the detached tree; or, should it just store the new children and wait until the tree is re-mounted before rendering them.
I can see rationales for both:
- If we render to a detached tree it should mostly work out, but some dom operations that may be done in refs may not like it and expect there to be a parent. Perhaps something like
.closestthat tries to crawl up to the root or dom operations that calculate layout. - If we don't render to a detached tree then React components inside that tree will not be updated. If this includes a props change that affects something like Redux/Flux subscriptions the Component may continue to work with out of date subscriptions, which could result in runtime errors as the Component tries to access data that doesn't exist anymore or is under a different name now.
I've added a possible alternative to .unmount(), a .keep() signal.
I'd like some feedback on that idea. If people like it I may refactor the RFC to use .keep() and make .unmount() the alternative.
I've also clarified the goals and explained why reparenting is useful even if we can't retain native state like focus and iframe contents and even when on React Native where there's not necessarily any concept of reparenting at the native View level.
I'm not sure what sort of feedback to give, as I am no expert on React (especially its internals) nor do I have extensive ideas on Reparenting. I have but two use cases that would warrant Reparenting, one of which is also solvable using Portals (or so I presume: I have not actually put this to the test).
Both of my use cases, however, concern with immediate reuse of subtrees, with the subtrees only changing places and never being explicitly detached from the DOM for any length of time longer than what it takes for them to reattach to a new parent.
Coming from that background the first impression I get from the createReparent API is that it is too burdensome for my use cases. My use cases do not care to explicitly .keep() instances around, nor do they particularly care about being bound to some component higher in the tree (though it is of course true that they are all children of the same node eventually, although for some of them that least common ancestor is actually the true root of the whole tree). They only care about persisting the time between two rendering cycles, essentially. Implicit unmounting done by React would be good enough for me, and it would save me the trouble of hoisting the Reparent creation into a higher component etc. (I'm referring to eg. some global key generation mechanism or such)
It likewise feels to me like holding onto a detached DOM tree for longer than the time between two rendering cycles would be perhaps antithetical to React's design principles. Explicit .keep() is probably better than implicit persistence until .unmount() is called or the Reparent is unmounted, as the latter would open some doors to memory leakage from forgotten Reparents. Nevertheless, the .keep() call begs the question of why not just insert the component into the tree and set its display property to none? What can we achieve with the detaching and reattaching, beyond what is possible with reparenting and just plain old attributes?
If you detach in order to simplify your DOM tree, but intend to return this component to the same place in the tree, then you're probably doing something wrong and should be using an attribute, not a Reparent.
If you detach in order to move the subtree to some other location, but don't want to render it there yet, then you definitely do need a Reparent, but why do you delay the insertion into DOM? If you do not want to show the subtree there just yet, use an attribute.
In short, I cannot see a good reason to detach a tree and keep it that way until some grand designs come to pass.
Yet, it is more than possible that I am simply ignoring some obvious benefits. Let it also be made clear that I am 100% with you on the Reparenting API: Something like this is really, really needed. It is an obscene state of things that a tree diffing algorithm based system such as React does not have some way to provide hints to the heuristics with the aim of making performant subtree comparison and moving possible.
My use cases do not care to explicitly .keep() instances around, nor do they particularly care about being bound to some component higher in the tree (though it is of course true that they are all children of the same node eventually, although for some of them that least common ancestor is actually the true root of the whole tree).
You are not required to use .keep(). The purpose of keep is to explicitly tell React that a reparent is still being held on to even if you don't actually render it. The this.reparent(content) also implicitly tells React that you are using the reparent. Basically, calling this.reparent.keep() is the same as calling this.reparent(content), except you don't have to provide new contents for the reparent. So your simple use cases remain simple, but .keep() is available for advanced users that need it.
For example you may not use it, but a library (maybe a DnD one) you are using may use it under the hood and you'll benefit from it's existence without having to see the complexity that is benefiting you.
And if you aren't hoisting (i.e. you are moving things around in the same component you create the reparent) and aren't removing things dynamically. Then the auto-unmount on owner unmount behaviour just takes care of things for you. You don't need to care about hoisting, keep/unmount, or anything complex.
See the Layout example for the simplest use case.
Reparents. Nevertheless, the .keep() call begs the question of why not just insert the component into the tree and set its display property to none? What can we achieve with the detaching and reattaching, beyond what is possible with reparenting and just plain old attributes?
- If the component is heavy, maybe you personally just want to avoid it's presence in the dom until needed but don't want to recreate it from scratch.
- Maybe you have a complex use case where something is being moved async, the source parent has been deleted, but the node for the destination has not been created yet. In that case you can either just do the simple act of calling
.keep()(or not calling.unmount()at this point), or you can add a bunch of code to your render to add adisplay: nonediv to your root (just for the explicit purpose of rendering something hidden that doesn't need to be rendered anywhere), then render the reparent there... although sincethis.theReparent(content)expects content you'll need to fine a way to get that if it's not part of the parent. - Maybe you have a complex stylesheet with lots of
:first-child,:last-child, and:nth-child-of-type. And if you simplydisplay: none;part of your content instead of detaching the content, then all the styles on your site will look messed up because:first-childis applying margins and other styles to a box you can't see instead of applying it to the first box in the list. - Maybe you're on native and working with a view where it's not easy to just try to hide it by style, but it is easy to detach the state tree. The views them selves may be destroyed and recreated later instead of actually detached if the native platform doesn't support detaching things, but at least any React components in that state tree are not unmounted so you don't lose your state (which is even more important).
Ultimately we are making a framework-level feature so we should not be dictating to the user whether we grant them permission to use the DOM concept of detached trees or not.
What I meant with the complexity of the createReparent, and .keep() is that for my simple use case, I'd love to just have eg. React.createElement("div", { key: React.createReparentableKey(idParam) }) and not worry about anything beyond that: No extra API needed from the user point of view, this div would be moved from one tree location to the next if React finds that this particular reparentable key moves. But, of course, that is a very limited use case. (Also, please don't read too deep into that example. I know it is quite unworkable for multiple reasons, like the generation of a new key every render cycle etc. It is not meant to be a proposal, but purely an outline of what sort of API would be the least invasive possible to enable my use cases.)
Your answer does quite well outline why my simple use cases do not represent the full power of this RCFS. Especially the async rendering and the native cases strike me as important. Heaviness of a subtree in a DOM tree (or native component tree I guess) doesn't sound like a likely situation, and the CSS solutions sound quite a bit like something that would be done for lists, where likely the inner state of the items would not be important. I'm sure these two have viable cases to be made as well, though.
Anyhow, async rendering of parents for the reparentable subtrees is truly a striking concept here that may warrant an example (If there wasn't one already. I admit it's been a few days since I last read the full text.) And of course the persisting of React subtree component states, even in native environments where hiding doesn't exist and detaching is the normal way of doing things.
Consider my feedback to be quite thoroughly trounced. I'll read a bit more into the proposal at a later time and try to come up with more feedback if I can find something to comment on.
What I meant with the complexity of the createReparent, and .keep() is that for my simple use case, I'd love to just have eg. React.createElement("div", { key: React.createReparentableKey(idParam) }) and not worry about anything beyond that: No extra API needed from the user point of view, this div would be moved from one tree location to the next if React finds that this particular reparentable key moves. But, of course, that is a very limited use case
Most of the simple use cases I've seen where you are moving things around involve a component where you already have a component it's reasonably possible to just put createReparent into, and the createKey technique (which has downsides) isn't needed. However I would welcome more examples of code where people need reparenting.
Side note, if we go with the .keep() semantics where if you don't call either .keep() or the reparent function with content in a render(), then React unmounts it like it normally would. Then it might be possible to write you own simple reparentable(idParam, <div />) that you can use without creating individual reparents.
const roots = Object.create(null);
function reparentable(key, content) {
if ( !roots[key] ) {
roots[key] = React.createReparent(root); // Either expose your global app's instance and use it here, or perhaps we could just make it valid to pass `null` to explicitly say you are handling auto-unmount yourself.
}
return roots[key](content);
}
The reparent itself would never go away since you're holding on to it, so you'd have to accept that fact and not use it for something like autoincrementing ids. But things would unmount when you are not using them. And things would move around instead of being recreated when you use the same id string.
Or, with less side effects. You could probably create a library that works like this using context + reparents.
const Application = () => (
<ReparentProvider>
<MyPage />
<ReparentProvider>
);
const MyPage = () => {
<Reparentable id='unique id key'>
<MyContent />
</Reparentable>
};
i.e. As long as you put a provider at the top of your application, you could use a simple component that only needs to know what the globally unique ID is.
Or if you want the ease of a provider, but actually want semi-unique keys. Like a database id where you know the id is unique within the database items, but you don't know that it's unique amongst all the keys you use for reparents.
import createReparentContext from 'some-poorly-named-library-based-on-reparent';
const DatabaseItemReparent = createReparentContext('human readable id for debugging');
// Alternatively you could do something like `{Provider: DatabaseItemRoot, Consumer: DatabaseItem}`, or find some other glossary term like "Roots" that make more sense
const MyPage = () => (
<DatabaseItemReparent.Provider>
<MyContent />
<DatabaseItemReparent.Provider>
);
const MyContent = () => {
<DatabaseItemReparent id={item.databaseId}>
<MyDatabaseItemContents databaseItem={item} />
</DatabaseItemReparent>
};
Actually this brings up a separate question. If we use the .keep() behaviour and unmount when no render has an association to the reparent, then do we need the auto-unmount on owner unmount behaviour? We needed it with .unmount() because we needed an easy way to know when a reparent was no longer in use, which in the simplest use case is when the component that created it no longer exists. But if reparents are automatically unmounted when you don't tell React you are still using them, then we don't actually need the this/owner argument to createReparent.
Actually this brings up a separate question. If we use the .keep() behaviour and unmount when no render has an association to the reparent, then do we need the auto-unmount on owner unmount behaviour? We needed it with .unmount() because we needed an easy way to know when a reparent was no longer in use, which in the simplest use case is when the component that created it no longer exists. But if reparents are automatically unmounted when you don't tell React you are still using them, then we don't actually need the this/owner argument to createReparent.
Indeed, the explicit .unmount() calls could be rid of and, I believe, should be rid of. The implicit unmounting of subtrees that are not explicitly in use is, after all, a wonderful way to make sure that we are not leaving behind any forgotten reparents. So at every rendering cycle, a reparent would always appear in some render function, either as a return value or called for safekeeping.
This would (probably) make the simple use cases as effortless as possible, while at the same time enabling the complicated long-detachment scenarios without express danger of leaking memory.
Can you think of any downsides to this approach, except the obvious one of having to always call the .keep() method? (This is not, in my opinion, a downside at all as it feels like a very React-like way to solve the problem.)
I like the idea of moving nodes around, but this seems as a needless overhead for the developer. Why not introduce new react-specific attribute, similar to the key that will tell React that this is the same node?
I like the idea of moving nodes around, but this seems as a needless overhead for the developer. Why not introduce new react-specific attribute, similar to the key that will tell React that this is the same node?
This is mostly explained in the Requirements section. Here are a few points:
- No other React API has the concept of a globally unique string.
- In fact it's currently pretty messy to handle unique
ids within a isomorphic React app and we have RFCs for semi-complex APIs to solve this.
- In fact it's currently pretty messy to handle unique
- Using globally unique keys makes using reparents harder in libraries since you have to ensure that your keys will not conflict with any the user makes.
- Using globally unique keys makes it harder to use things like database IDs for the unique key. You can guarantee that this ID is unique within the set of data you want to be able to move around, but you cannot guarantee that the ID is 100% unique among all possible reparentable nodes. Thus you have to pad it with uniqueness that shouldn't be necessary for the type of reparenting you are doing.
- If we use keys it is not possible to hold on to detached nodes. There are legitimate reasons to want to be able to intentionally detach a tree temporarily. And there are legitimate cases where a tree momentarily may not be present in the tree during a move but you do not want it to be unmounted (and this type of async move may become more common after the future async rendering mode is introduced).
I'm currently building a web browser-like app in react native. One of the requirements is to be able to switch from viewing a single tab to viewing all tabs in an animated scroller - at the moment, due to the inability re-parent, switching between views causes tabs to be re-mounted from scratch, loosing any and all state. So re-parenting would be useful for me.
Great work on the RFC @dantman. 🙌 That'd definitely be useful for us at @PSPDFKit as well!
The .keep API makes sense as it must be possible to retain elements even if they aren’t rendered it in the current tree. However it seems a bit counter-intuitive that the reperantable element will also be kept in the following example:
class DetachableTree extends Component {
reparent = React.createReparent(this);
render() {
const content = this.reparent(this.props.children);
return this.props.show && content;
}
}
I think it's also against the design principles of React that the render method has side effects. In this case, the render method might return null but we still have to track that we want to keep the reperantable - thus we would need to save that information as a side effect. Maybe this can be improved by rendering a special component that describes this keeping state?
Another problem is that the lack of a context makes it possible to create a reparent deep in the tree and pass it upwards to render it in the root. That's something that's not possible with the context API for example and I'm not sure if this causes any implications.
I also fear that the id passing might be very confusing for developers that use it. Your examples make use of getDerivedStateFromProps twice which smells because that method should rarely be used at all. I personally prefer your createReparentContext to solve this. In this API, you create a context/repereantable pair similar to the new context API. This will drastically reduce the chance of an id clash since you are in total control of the context. Third party APIs can never clash with your repereantable contexts as well.
Here’s one idea that I have in mind that tries to cope with the problems I outlined above. I decided to migrate your TemplateWidget example to show you how you can get rid of gDSFP:
TemplateWidget using createReparentContext
const WidgetReparentContext = React.createReparentContext();
function TemplateWidget({ id }) {
return (
<WidgetReparentContext.Reparentable id={id}>
<p>Hello from widget {id}</p>
</WidgetReparentContext.Reparentable>
);
}
function Section({ title, widgets }) {
return (
<div>
<h2>{title}</h2>
{widgets.map(widget => <TemplateWidget {...widget} key={widget.id} />)}
</div>
);
}
function Template({ sections }) {
return (
<WidgetReparentContext.Provider>
{sections.map(section => {
<Section {...section} key={section.id} />;
})}
{keep}
</WidgetReparentContext.Provider>
);
}
Nothing special about this so far, this is basically your createReparentContext idea. One thing to notice here is that your solution does also keep widgets mounted that were once passed in as props but are no longer. I think that this use case can be further generalized into custom cache strategies which must know which items are added in order to know how to cache them.
To solve this, I assume that we could add a callback to the ReparentContext.Provider which gets called whenever a reparentable key is rendered. A regular React component could then collect all of those and render special "keep" placeholders so that they are always part of the render results:
TemplateWidget using createReparentContext with onReparentableRendered
const WidgetReparentContext = React.createReparentContext();
function TemplateWidget({ id }) {
return (
<WidgetReparentContext.Reparentable id={id}>
<p>Hello from widget {id}</p>
</WidgetReparentContext.Reparentable>
);
}
function Section({ title, widgets }) {
return (
<div>
<h2>{title}</h2>
{widgets.map(widget => <TemplateWidget {...widget} key={widget.id} />)}
</div>
);
}
class Template extends React.Component {
state = { keptReparentables: [] };
// In this example we use the `onReparentableRendered` callback to be notified
// whenever a new reparentable is rendered. This will easily allow more
// advanced caching systems (e.g timeout- or LRU-based) to be implemented
// as React components.
addReparentable = ({ id }) => {
this.setState(state => {
// We already cache this reparentable so there's nothing to do.
if (state.keptReparentables.includes(id)) {
return null;
}
// We add this to our kept reparentable list.
return { keptReparentables: [...state.keptReparentables, id] };
});
};
render() {
const { sections } = this.props;
const { keptReparentables } = this.state;
return (
<WidgetReparentContext.Provider
onReparentableRendered={this.addReparentable}
>
{sections.map(section => <Section {...section} key={section.id} />)}
{/* A special kept flag will tell React to keep this reparentable around
if it exists without updating properties. */}
{keptReparentables.map(id => (
<WidgetReparentContext.Reparentable id={id} keep />
))}
</WidgetReparentContext.Provider>
);
}
}
I'd love to hear your thoughts on this API 😊 I also think that this API can be implemented in userland if we have https://github.com/facebook/react/issues/13044 (well, for ReactDOM only at least and without calling getSnapshotBeforeUpdate/componentDidUpdate which you propose as well).
I think it's also against the design principles of React that the render method has side effects. In this case, the render method might return null but we still have to track that we want to keep the reperantable - thus we would need to save that information as a side effect. Maybe this can be improved by rendering a special component that describes this keeping state?
This is correct, we do have to accept that in this case not only is there an effect from a return from render but also a .keep()/Reparent() in render. I am half on/half off with this idea though, while technically true it being a side effect, in a way we are just tracking references in render() and GCing things when references disappear which doesn't feel as strange as other types of side effect. I did consider requiring that this be passed to .keep() if it turned out this behaviour couldn't be implemented in React.
Fragments do make it possible to, use some sort of internal reference tracking element where we pass the reparent to it to keep the reference without it being rendered, without it messing with the dom. But I'm very hesitant to use that pattern given how horrible it looks to use that type of pattern. How it limits how render() can be written, in some cases forcing you to rewrite what could be a conditional if (...) { return ...; } into an if (...) { result = ...; } return <>{/* code to keep references to reparents */}{result}<>;. And I also feel a little awkward about how .keep() is designed to allow you to keep a reference to something while still rendering it somewhere else, something that feels at odds with rendering a special component because you are returning a reparent in 2 places in JSX, it just happens that one of those places is actually a special type of component that doesn't actually render the reparent using a loophole to bypass the "a reparent may only be rendered in one place" rule.
Another problem is that the lack of a context makes it possible to create a reparent deep in the tree and pass it upwards to render it in the root. That's something that's not possible with the context API for example and I'm not sure if this causes any implications.
I suppose someone ingenious might be able to find a way to pass a reparent upwards and find a way to use a few tricks to make a reparent render the component that first created the reparent as as child. Although I don't believe this is possible without the use of a second reparent that technically means the component wouldn't actually be a child of the reparent created in the component.
I'm also not sure this is even a problem, as with the new .keep() API I believe a Reparent is now pretty much disconnected from the component that creates it. We only opt-in to patterns like that because it's useful to have a component keep references to a series of reparents so they don't unmount and pass those reparents to its children.
Also worth noting, it's not directly within the React tree but the Portal API does allow you to render something in the root from a child component of the root.
I also fear that the id passing might be very confusing for developers that use it. Your examples make use of getDerivedStateFromProps twice which smells because that method should rarely be used at all. I personally prefer your createReparentContext to solve this. In this API, you create a context/repereantable pair similar to the new context API. This will drastically reduce the chance of an id clash since you are in total control of the context. Third party APIs can never clash with your repereantable contexts as well.
The IDs are purely an implementation detail of the example, they aren't required of the API. If someone wants they are free to use a Map and object keys in theirs. Id clashes are also a non-issue, the IDs only matter within the scope of the context created by the user in the example. i.e. You can create another context containing reparents for something else and use the same ids, there will be no clash because they are separate contexts – the IDs do not need to be globally unique.
Honestly the only reason that example is ID based is because the application I am currently working on happens to use IDs. I needed an example of that kind of pattern of passing a dynamic set of reparents to descendents via context, so I just used what I had on hand. That example is basically just a pattern I'm actually using in the application I'm currently working on right now, just stripped down to the most basic/generic parts so I'm not sharing any client code.
Also I think the examples originally needed getDerivedStateFromProps a bit more in the pre-keep version of the rfc. Now that we have keep, I have a feeling that you could probably refactor that example to work using very very minor memoization in render(). Also that example is an example of one of the most extreme most advanced ways you can use a reparent, the example kind of is the "rare" situation where you do want gDSFP.
Speaking of userland things, I am aiming for an api that is as flexible and supporting of advanced cases as possible. As I expect that a simple API will not cover all the use cases people want. And if we implement an API that is advanced/flexible enough people will be free to write libraries that expose simple APIs for using reparents. This will let different use cases for reparents be supported by separate libraries that each have an API that is simplest to use for the use case they target. And will allow anyone with a use case too advanced for a library to handle to use the built in API. In particular, that createReparentContext you like is an example of an idea for a 3rd party API someone might want to make which can be implemented using the reparent API in the RFC.
Thank you for the response!
Your point that the id is only artificial for the example and might not always be present is good although I think that even if we allow the usage of an id we'd still not be incompatible with use cases where this is not required. One could simply implement their own createKey() method that returns (++i).toString() which is unique in the context they created (that, of course, only works if we go with the createReparentContext() idea). I think that both APIs are compatible and we should find out which use case is more common. In all the cases we at @PSPDFKit would use reparenting, we always have an obvious id candidate at hand and would be able to just use that. It's correct that such a solution could be implemented in userland so I don't care too much.
However I'm still not keen on the idea that we're sacrificing the purity of the render method because we have to track keep() calls. I agree that your API looks more compact but would it make it clear that, for example, you'd have to call setState() and force a re-render to be able to "unkeep" reparentables? I think a React component would make that more obvious because people using React are already familiar with that concept.
Also, as you said, the use case where we'd require to return a fragment of "keep" elements "is an example of one of the most extreme most advanced ways you can use a reparent". So I assume for most users this will be hidden behind a library that manages different reparentable caches.
And I also feel a little awkward about how .keep() is designed to allow you to keep a reference to something while still rendering it somewhere else, something that feels at odds with rendering a special component because you are returning a reparent in 2 places in JSX, it just happens that one of those places is actually a special type of component that doesn't actually render the reparent using a loophole to bypass the "a reparent may only be rendered in one place" rule.
That can easily be solved by returning three elements from the createReparentContext() API and not calling the special loophole reparentable at all:
const { Provider, Reparentable, Keep } = createReparentContext();
function SomeComponent ({children}) {
return (
<Provider>
{children}
{/* Always keep some-reparentable around */}
<Keep id="some-reparentable" />
</Provider>
);
}
To make the React.Fragment obsolete it could also accept an array of ids:
<Keep ids={["some-reparentable", "some-other-reparentable"]} />
Another thing that is impossible with your API (which I just found out now that I sketched the above example): It would require the use of a class component for the this to be passed into createReparent().
Edit: Another example where the .keep() API is insufficient is when you think about unit testing using techniques like the shallow renderer. How would you write an assertion that verifies that .keep() is called indeed? We'd probably have to expose a custom API for that or you'd need to mock createReparent(). Returning an React component from render() makes this a lot simpler - you could even use Jest snapshots!
However I'm still not keen on the idea that we're sacrificing the purity of the render method because we have to track keep() calls. I agree that your API looks more compact but would it make it clear that, for example, you'd have to call setState() and force a re-render to be able to "unkeep" reparentables? I think a React component would make that more obvious because people using React are already familiar with that concept.
Is this really something that strange?
- You are explicitly calling a method that says it will keep a reference to the reparent. And in some cases you are explicitly calling a
.keep()method, whose sole purpose is to keep a reference and only reason you'd call it is if you knew what it did. - I'm also not sure forcing a re-render with setState is that strange either. If you keep/render children for a reparent in a render you've essentially "used" the reparent in a render(), it doesn't seem that strange that something you used in a render() won't go away until that component is rendered again.
That can easily be solved by returning three elements from the createReparentContext() API and not calling the special loophole reparentable at all:
Ok that covers the strangeness with a different API. Though I don't really like this id based pattern because there are also use cases for singular reparents.
Another thing that is impossible with your API (which I just found out now that I sketched the above example): It would require the use of a class component for the this to be passed into createReparent().
Oh, whoops. I didn't realize that my examples and my protocol still listed this/Component in them. Ignore those, the rewrite to use .keep() was so big I missed things. this was required in the original .unmount() based API because we needed an "owner" so we could implicitly unmount when the owner unmounted. With the .keep() we do not need to know the owner because we only have to keep track of what components are using the reparent in order to know when to umount its contents.
Every time an example uses React.createReparent(this) it should actually be React.createReparent() now.
Another example where the .keep() API is insufficient is when you think about unit testing using techniques like the shallow renderer. How would you write an assertion that verifies that .keep() is called indeed?
It's possible you may need a bit more than a shallow render, I'm not sure how shallow the shallow renderer is. But you can test the effects of keep by watching whether a component instance unmounts or not. I think this is justified because you are not testing the react tree but testing the effects of a mounting behaviour.
Something along the lines of:
let mounted = false;
class Foo extends Component {
componentDidMount() {
mounted = true;
}
componentWillUnmount() {
mounted = false;
}
}
const reparent = React.createReparent();
function Bar({show, children}) {
reparent.keep();
return show ? reparent(children) : null;
}
renderer.render(<Bar show><Foo /></Bar>);
// wait
expect(mounted).toBe(true, 'Foo should be mounted when rendered through a reparent');
renderer.render(<Bar hide><Foo /></Bar>);
// wait
expect(mounted).toBe(true, 'Foo should not unmount even when not rendered, becase .keep() was called');
renderer.render(<Bar show></Bar>);
// wait
expect(mounted).toBe(false, "Foo should unmount when it is no longer part of the reparent's children");
+1 to not particularly liking the id based approach. I find it confusing.
If the side-effect keep-function is seen as a problem, would making keep into a React Component be an acceptable alternative? I think this has been partially in discussion between you already, but I wasn't exactly sure if it was the same as I was thinking about.
So essentially something like a React.Fragment, but instead of allowing a grouped-together set it would act as the keep-call for one or more Reparents inside it. This admittedly adds complexity to the already relatively complex API but would remove the side-effects and would make the API a reasonably accurate rendition of the actual resulting DOM: The main DOM tree expands as the React tree does, while the Reparent components inside the Keep component are still in existence in the DOM, just in a detached state. The component could even be named with something to that effect, like Detached or I don't know.
Sorry if this is just rehashing the same ideas already given and shot down.
EDIT: Whoops, I'm really stupid. The Keep Component was def. proposed earlier, just with the id based system. I'd definitely prefer it as a Fragment-like component.
@aapoalas You're absolutely not stupid, thank you for helping us out here 🙌
So just to clarify, you have something like this in mind, right?
const reparent = React.createReparent();
function SomeComponent() {
return (
<React.Fragment>
{reparent(<p>Some DOM</p>)}
<React.Detached>
{reparent()}
</React.Detached>
</React.Fragment>
);
}
I think one problem here is the same that @dantman pointed out earlier: It's hard to see why we're not allowed to render the same reparent() twice inside the React tree but it's ok when one of them is wrapped inside a <React.Detached>. I'm quoting here because I found his words clearer:
And I also feel a little awkward about how .keep() is designed to allow you to keep a reference to something while still rendering it somewhere else, something that feels at odds with rendering a special component because you are returning a reparent in 2 places in JSX, it just happens that one of those places is actually a special type of component that doesn't actually render the reparent using a loophole to bypass the "a reparent may only be rendered in one place" rule.
What I like about this idea though is that the return value of createReparent() can now easily be a component itself which makes it a mot more intuitive to use:
const Reparent = React.createReparent();
function SomeComponent() {
return (
<React.Fragment>
<Reparent><p>Some DOM</p></Reparent>
<React.Detached>
<Reparent />
</React.Detached>
</React.Fragment>
);
}
Edit: Another problem with <React.Detached> is that it's not clear why we can only mount <Reparent /> inside, not any other React component. That seems awkward.
Edit2: There are also some other problems with my context based approach that evolve when we have more than one context mounted in the react tree and unmount only one or when we render the context within the reparentable (as opposed to the other way around). I think that the unique non-key solution will fix this as the reparentable will simply be allowed in the whole React tree.
What I like about this idea though is that the return value of createReparent() can now easily be a component itself which makes it a mot more intuitive to use:
That's an interesting idea, one caveat though. How do we differentiate between rendering <Reparent /> as a "keep" and rendering <Reparent></Reparent> with empty children? We do need to allow a reparent to be rendered with empty children. It would be really confusing if the answer to that was "<Reparent /> behaves differently when it is a child of <React.Detached />.
I still don't quite like the idea of an element based API for reparents. But this could work for one:
const Reparent = React.createReparent();
function SomeComponent() {
return (
<React.Fragment>
<Reparent.Reference />
<Reparent><p>Some DOM</p></Reparent>
</React.Fragment>
);
}
We can use a different name than .Reference if someone can come up with one. But this would also make <React.Detached /> unnecessary. Rendering the Reparent itself would render it and its contents. But rendering the <Reparent.Reference /> would just be a reference to the reparent and wouldn't change it's contents.
Though I still do not like the idea that a complex branching render() would have to be refactored so it can return elements that aren't actually elements and aren't actually rendered in order to keep references to something.
What if you could pass a component to createReparent?
const reparent = React.createReparent(() => <p>Some DOM</p>)
function SomeComponent() {
return (
<React.Fragment>
<Reparent />
</React.Fragment>
)
}
