The child management in this guide is misleading
gaearon opened this issue · 5 comments
Please see this issue: facebook/react#13006.
Children aren't meant to be manually managed in an array. Instead, the idea is that you should call underlying library's appendChild and similar methods in your appendChild.
Declaring classes and keeping track of children in arrays and then calling render kind of goes against that because you're ignoring the valuable information React gives you (which child was added/removed) and then somehow try to reconstruct it later.
I temporarily removed the link to this tutorial from the README of the reconciler package but happy to re-add it if these issues are addressed. Also happy to answer more questions.
One interesting aspect here is that if your rendering target doesn't provide "mutative" methods like appendChild, and instead only lets you replace the whole "scene" at once, you might want to use the "persistent" renderer mode instead. Here's an example host config for persistent renderer.
Thanks for letting me know about this. Just updated the tutorial and also the code. For reference, here is the link to a custom component in docx environment that uses platform specific function to append child nodes instead of storing internally inside children array.
Do we need render at all? Can you explain more about what it does?
I’m worried it’s confusing people by association with the component render function.
Another thing that’ll need updating is that the host config format changed a bit. Instead of the separate mutation object we now put all methods at the top, and have a property called supportsMutation that needs to be true in mutating renderers.
Oh! I think we can drop render method from the classes since it is not required for the example in the guide. Though I've used it earlier when I created redocx where it fulfilled a specific use case.
Since now we are appending the child nodes using the platform specific mutative APIs, render method is useless here. I didn't changed the implementation for parsing the nodes, where render method is needed and that's why I didn't removed it.
Anyway, I'll update the code and guide accordingly. Thanks for clarification!
Updated the guide with best practices 😄