Patching concerns: Missing node `parent` and consistent invocation of `created` hook

[brandonpayton]

Hi @jails, thanks for your work on this library. The code is quite readable, and I'm enjoying working with it.

I'm creating this issue to discuss two issues because they both involve the same patching code, and it would be good to get your thoughts before splitting to a second issue.

Issues

1. Patching may render nodes with no parent
   • I believe this and this from src/tree/patch.js combined with Tag#render, which now takes two arguments, mean that some nodes inserted by patch are created without a node parent.
2. Tag's created hook is called inconsistently relative to when its element is inserted in the DOM.
   • When a container is provided to Tag#render, the created hook is called after the node's element is inserted in the DOM, but when Tag#render appends itself to a document fragment, created is called before the node is added to the DOM document by tree/patch.
   • This matters because created hooks may need to do things like take layout measurements which require being part of the document flow.
   • There may be a place for both pre and post insertion hooks, but I believe at least a post insertion hook is necessary for the reason above.

     Discussion

In case we both agree on the above, here are some thoughts and questions:

1. Would it be better to separate the concerns of parent, rendering, and insertion?
2. Who should be responsible for setting a node's parent?
   • Currently, I am thinking a node's constructor should be responsible for setting itself as the parent of its children. Derived information such as an inherited namespace value could be determined on render if needed.
   • I think the current approach may be workable, but it seems strange to set parent as a side-effect of the render operation.
3. It would be simpler and possibly more natural for render to take no arguments.
4. It would be good for DOM insertion to be encapsulated by tree or node modules rather than spread across both.
   • Where should this responsibility go?
   • Making it a tree responsibility
     • Pros
     • Limits actual calls to DOM appendChild and insertBefore to tree modules
     • Cons
     • Makes the tree code DOM specific
     • Introduces an awkwardness with executing a post-insertion hook
   • Making it a node responsibility
     • Pros
     • A step toward decoupling tree modules from the DOM and allowing use with other platforms
     • Better encapsulates concerns of particular node types
     • Gives node control over when post-insertion hook is executed
       • Counter: I had been thinking we'd want this for web components if dependencies such as embedded CSS @imports are not sychronously, deterministically resolved, but if that was the case, you might want to do the same thing with an <img> which makes this thought sound less reasonable.
     • Cons
     • Additional methods required to create new node types, likely one for append and one for insertBefore.
       • Counter: They should generally be simple to implement.

There may be more pros and cons, but that is what I'm able to think of at the moment.

What are you thoughts? I realize this is quite an opinionated brain dump.

Contributions

Are you up for taking contributions? If so, let me know what approach you'd like to take with these things (if any), and I am happy to do the work.

Thank you!

[jails]

Thanks for reporting this issue !

Issues

1. Indeed, something is messed up here. render(parent) should be render(container, parent). Would be great to be able to put this issue in evidence in a spec. Since you stumbled on this issue you probably know how we can reproduce it ?
2. Fair enough, this document.createDocumentFragment() was more a hack since IE was slow as hell when trying to add nodes to the DOM directly one by one. created should be a post creation hook and it's indeed an issue with fragments. And I've no idea how to solve that since a fragment can live in memory and it's not possible to catch when it'll be effectively added in the DOM.

Discussion

1. 1 & 2 & 3. Imo all methods are valids here. I've no clear position on this subject. I see dom-layer as a low level library. I'm personnaly using it in a higher abstraction so I don't need to take care about the low level API. I think it was the strategy which minimizes "ugly code" but I may be wrong.
2. Not sure to get your question but imo DOM insertion (ie all document.appendChild()) should be a tag concern. Indeed tags should be responsible of rendering itself. This way you can create your own tags as well of even high level tags (like widget for example) and have them rendered correctly in the DOM. Imo the role of the tree is to simply call render() methods, it's should need to know how each tag should be rendered.

Contributions

Sure I'm always open to new contributions when they make sense. I'll do my best to help if I can.

[brandonpayton]

Cool. I think we are mostly on the same page.

I'll create a spec showing each issue along with separate GH issues and then a PR or two with fixes. I'll leave this issue open until then.

[brandonpayton]

Hi @jails, after digging deeper, I began to think that I'd like to do things a bit differently than dom-layer, so I've been playing around with creating an even simpler virtual DOM library using some of the same ideas. I planned to show you my thoughts at the very least, but due to a change in technical direction at work, I do not have much time to spend on either effort. However, I have repro steps for missing parent that I'll file under a dedicated issue. I opened a dedicated issue about the created hook as well.

Thank you for sharing your work, and I'll share mine if I get time to test and publish.