Document Descendants and Children Complete

[brianmhunt]

@mbest - I've been trying to implement the DescendantsComplete and ChildrenComplete events (i.e. pass all the unit tests), and it's been taking a bit longer than it should because TKO does a few internals differently now and I haven't fully figured out how these events should work.

It would help me a ton if you were able to put together a bit of documentation (either formally or informally here) that relayed how they act in the various circumstances. It's clearly well thought out, but because the TKO internals are different the don't align exactly.

A high level description would go a long way, but some code-comments on the AsyncCompleteContext (why we need it, how it differs from a regular binding context) would go a long way and be much appreciated!

[mbest]

childrenComplete is triggered as soon as bindings are completed on the node's children. Completed means that applyBindings was called and returned for each node. It's always triggered synchronously from the point of view of the code that applies bindings to the child nodes. It will be triggered again if the node's children are re-bound (usually after being cleared and re-rendered). It may be triggered even if the node has no children, although in that case, the event won't be pushed to callbacks set in the node's binding. It's also triggered by the foreach binding each time it's updated even though not all children nodes are re-bound.

descendantsComplete is triggered when all the node's descendant nodes are completed (they've all triggered the childrenComplete event). If any of the descendant nodes indicates that it will complete its children asynchronously, the event will be delayed until that node's children are completed. In the case where all descendants are bound and completed synchronously, it will be triggered synchronously.

[brianmhunt]

Thanks Michael. Should descendantsComplete trigger repeatedly e.g. with foreach changes?

[mbest]

Should descendantsComplete trigger repeatedly e.g. with foreach changes?

I think it will since it's triggered by the childrenComplete event. I just noticed that I don't have any tests for these events and foreach. I'll have to work on that.

[brianmhunt]

Got it, thanks.

Relatedly, in tko the applyBindings returns an instance of a class called BindingResult. I'm mulling what it should return, but in essence I'm hoping it can expose the binding results, events, and errors in a way that helps debugging and makes KO easier to implement in certain use-cases. Any thoughts on it?

[mbest]

Here's more on how the nodes need to interact to trigger a descendantsComplete event:

1. A node that will complete asynchronously needs to mark itself as such; later it will need to clear that mark when it's complete.
2. A marked node is considered complete if it's removed or cleaned (the mark is cleared).
3. A node that will trigger a descendantsComplete event needs to wait for all descendant nodes to be complete. It can trigger the event once it has applied bindings to all of its child nodes and it has no descendant nodes that are marked as completing asynchronously.
4. The event should be triggered as soon as possible (preferably synchronously) when all descendants are complete.

[mbest]

BindingResult is a good idea and I think you've got the essential stuff in there now.

[brianmhunt]

Thanks Michael, that's very helpful. Glad you like the BindingResult idea.

[mbest]

The way I originally implemented the descendantsComplete logic went like this:

1. A node that will complete asynchronously marked itself as such by setting a domData value that included a way for ancestor nodes to register as waiting for it.
2. When the node was complete (descendantsComplete itself), it signaled to each of the waiting nodes that it was complete.
3. A node with a registered descendantsComplete callback or that had been marked as above would, after the childrenComplete event, iterate through all of its descendant nodes and register to wait for any marked ones.

I didn't like the fact that this method required iterating through all of the descendant nodes, but I also wanted to avoid having all nodes keep track of such things. So that led me to create AsyncCompleteContext as a way for the descendant nodes to register themselves with their closest "interested" ancestor node.

[brianmhunt]

I'll mark this as closed; thanks for the excellent comments @mbest, I think I've got it nailed.

TKO is down to 9 failing tests. 🍻 !

[mbest]

@brianmhunt Do you think you could use bindingResult.completionPromise to handle the above in TKO instead of AsyncCompleteContext?

[brianmhunt]

@mbest I was planning that and that's how I had initially been digging into it, but it seemed to fail on some edge cases in the tests, mostly because completionPromise triggers only once and apparently I hadn't sorted the logic of when it should trigger/resolve. Which is why I started this thread. 😄

Generally speaking, for the binding process (i.e. applyBindings*) I'd prefer of returning promises since it's cleaner and more readily apparent what's going on, compared to events, and I suspect there'd be a big performance advantage since with the promises the events would not need to be triggered for every binding application.

Did you have something in mind? I didn't want to muddle with how Knockout is designed/working, but if you can help me get completionPromise working here, and update Knockout & tests accordingly, I think that'd be valuable.

[mbest]

Okay. I'll take a look when I have some time.

[brianmhunt]

@mbest One problem I'm having is that descendantsComplete is called only when an if condition becomes truthy (i.e. the tests Should call a descendantsComplete callback function and following in ifBehaviors).

What's the reason for deferring descendantsComplete until the conditional is truthy?

I'm not certain it's what users will expect, and it's not entirely consistent with application elsewhere (e.g. should for-loops wait until there's one item?) . However, I might not fully appreciate the rationale here. Grateful for your thoughts.

[mbest]

That's a good question. This came from the discussion in knockout/knockout#2342

I had forgotten about the treatment of the if binding while describing descendantsComplete above. The if binding (and ifnot and with) only trigger a childrenComplete when their values are truthy. Normally, this won't delay an ancestor's descendantsComplete because it doesn't actually check each descendant node. It only waits for descendants that are marked as such. There are two ways that a node is marked as possibly completing asynchronously. The first is if the control-flow binding sets the mark (by calling startPossiblyAsyncContentBinding, currently only component does this), and the other is if the node has a descendantsComplete binding (which calls startPossiblyAsyncContentBinding).

[mbest]

I'm going to revisit how it works with if. I'm thinking of adding a binding option named something like ifDelayedComplete, which if true will cause the if binding to delay notifying the childrenComplete and descendantComplete bindings until its value is true. Similar logic would apply to ifnot and with. @brianmhunt What do you think?

[brianmhunt]

I think that's a good idea @mbest. Perhaps completeOn: "render" or something like that.

[mbest]

I like that.