#414 if binding evaluating to false delays invocation of kodescendantscomplete

.changeset/binding-lifecycle-parity.md

@@ -0,0 +1,37 @@
+---
+"@tko/bind": minor
+"@tko/binding.if": minor
+"@tko/binding.component": minor
+"@tko/binding.core": minor
+---
+
+Restore Knockout 3.5 binding-lifecycle semantics for `descendantsComplete` and
+add `completeOn: "render"`.
+
+`@tko/bind` now owns the KO 3.5 async-completion bookkeeping
+(`bindingEvent.startPossiblyAsyncContentBinding` / `AsyncCompleteContext`).
+`childrenComplete` drives per-node tracking of pending async descendants, and
+`descendantsComplete` fires once a node's children **and** every
+asynchronously-completing descendant (components, conditionals) have bound.
+
+Behavior changes to be aware of:
+
+- **`descendantsComplete` re-arms per content cycle.** Previously it fired at
+  most once (when the initial binding pass settled), and never fired for a
+  conditional that started false. It now fires each time a node renders content
+  — a nested `if` becoming true re-fires the ancestor's `descendantsComplete`,
+  and toggling a conditional off then on fires it again. Consumers who attached
+  one-shot `descendantsComplete` callbacks may see additional calls.
+- **`completeOn: "render"`** is now supported on `if`/`ifnot`/`with`. It defers
+  the node's `childrenComplete` (and any ancestor `descendantsComplete`, or a
+  component's `koDescendantsComplete`) until the binding actually renders
+  content. `completeOn` is a reserved binding key (as in KO 3.5).
+- A component viewmodel's **`koDescendantsComplete`** now fires via the
+  element's `descendantsComplete` event, so inner components complete before
+  their outer component (KO 3.5 ordering), including across intermediate
+  bindings and several nesting layers.
+
+Fixes the root cause behind
+[#414](https://github.com/knockout/tko/issues/414): `koDescendantsComplete` is
+no longer suppressed when a component template contains a conditional that
+starts false.

builds/knockout/spec/components/componentBindingBehaviors.js

@@ -295,9 +295,7 @@ describe('Components: Component binding', function () {
     expect(renderedCount).to.equal(1)
   })
 
-  // @mbest - This fails b/c `test-component`'s koDescendantsComplete is called
-  // after the test function completes.  Otherwise the result is correct.
-  it.skip("Inner components' koDescendantsComplete occurs before the outer component's", function () {
+  it("Inner components' koDescendantsComplete occurs before the outer component's", function () {
     after(function () {
       ko.components.unregister('sub-component')
     })
@@ -331,7 +329,7 @@ describe('Components: Component binding', function () {
     expect(renderedComponents).to.deep.equal(['sub-component1', 'sub-component2', 'test-component'])
   })
 
-  it.skip('koDescendantsComplete occurs after all inner components even if outer component is rendered synchronously', function () {
+  it('koDescendantsComplete occurs after all inner components even if outer component is rendered synchronously', function () {
     after(function () {
       ko.components.unregister('sub-component')
     })
@@ -400,7 +398,7 @@ describe('Components: Component binding', function () {
     expect(renderedComponents).to.deep.equal(['sub-component1', 'sub-component2', 'test-component'])
   })
 
-  it.skip('koDescendantsComplete waits for inner component to complete even if it is several layers down', function () {
+  it('koDescendantsComplete waits for inner component to complete even if it is several layers down', function () {
     after(function () {
       ko.components.unregister('sub-component')
     })
@@ -431,7 +429,7 @@ describe('Components: Component binding', function () {
     expect(renderedComponents).to.deep.equal(['sub-component1', 'test-component'])
   })
 
-  it.skip('koDescendantsComplete waits for inner components that are not yet loaded', function () {
+  it('koDescendantsComplete waits for inner components that are not yet loaded', function () {
     restoreAfter(window, 'require')
     after(function () {
       ko.components.unregister('sub-component')
@@ -997,7 +995,7 @@ describe('Components: Component binding', function () {
     expect(callbacks).to.deep.equal(1)
   })
 
-  it.skip("Does not call outer component's koDescendantsComplete function if an inner component is re-rendered", function () {
+  it("Does not call outer component's koDescendantsComplete function if an inner component is re-rendered", function () {
     after(function () {
       ko.components.unregister('sub-component')
     })

packages/bind/src/applyBindings.ts

@@ -98,34 +98,35 @@ function applyBindingsToDescendantsInternal(
 ) {
   let nextInQueue: ChildNode | null = virtualElements.firstChild(elementOrVirtualElement)
 
-  if (!nextInQueue) {
-    return
-  }
+  if (nextInQueue) {
+    let currentChild: Node | null
+    const provider = getBindingProvider()
+    const preprocessNode = provider.preprocessNode
+
+    // Preprocessing allows a binding provider to mutate a node before bindings are applied to it. For example it's
+    // possible to insert new siblings after it, and/or replace the node with a different one. This can be used to
+    // implement custom binding syntaxes, such as {{ value }} for string interpolation, or custom element types that
+    // trigger insertion of <template> contents at that point in the document.
+    if (preprocessNode) {
+      while ((currentChild = nextInQueue)) {
+        nextInQueue = virtualElements.nextSibling(currentChild)
+        preprocessNode.call(provider, currentChild)
+      }
 
-  let currentChild: Node | null
-  const provider = getBindingProvider()
-  const preprocessNode = provider.preprocessNode
+      // Reset nextInQueue for the next loop
+      nextInQueue = virtualElements.firstChild(elementOrVirtualElement)
+    }
 
-  // Preprocessing allows a binding provider to mutate a node before bindings are applied to it. For example it's
-  // possible to insert new siblings after it, and/or replace the node with a different one. This can be used to
-  // implement custom binding syntaxes, such as {{ value }} for string interpolation, or custom element types that
-  // trigger insertion of <template> contents at that point in the document.
-  if (preprocessNode) {
     while ((currentChild = nextInQueue)) {
+      // Keep a record of the next child *before* applying bindings, in case the binding removes the current child from its position
       nextInQueue = virtualElements.nextSibling(currentChild)
-      preprocessNode.call(provider, currentChild)
+      applyBindingsToNodeAndDescendantsInternal(bindingContext, currentChild, asyncBindingsApplied)
     }
-
-    // Reset nextInQueue for the next loop
-    nextInQueue = virtualElements.firstChild(elementOrVirtualElement)
-  }
-
-  while ((currentChild = nextInQueue)) {
-    // Keep a record of the next child *before* applying bindings, in case the binding removes the current child from its position
-    nextInQueue = virtualElements.nextSibling(currentChild)
-    applyBindingsToNodeAndDescendantsInternal(bindingContext, currentChild, asyncBindingsApplied)
   }
 
+  // Notify childrenComplete unconditionally (KO parity) — a node that renders no
+  // content still reached a completed state, which the async-completion
+  // bookkeeping and `childrenComplete` subscribers need to observe.
   bindingEvent.notify(elementOrVirtualElement, bindingEvent.childrenComplete)
 }
 
@@ -157,9 +158,9 @@ function applyBindingsToNodeAndDescendantsInternal(
     isElement || // Case (1)
     hasBindings(nodeVerified) // Case (2)
 
-  const { shouldBindDescendants }: any = shouldApplyBindings
+  const { shouldBindDescendants, bindingContextForDescendants }: any = shouldApplyBindings
     ? applyBindingsToNodeInternal(nodeVerified, null, bindingContext, asyncBindingsApplied)
-    : { shouldBindDescendants: true }
+    : { shouldBindDescendants: true, bindingContextForDescendants: bindingContext }
 
   if (shouldBindDescendants && !bindingDoesNotRecurseIntoElementTypes[tagNameLower(nodeVerified as Element)]) {
     // We're recursing automatically into (real or virtual) child nodes without changing binding contexts. So,
@@ -168,7 +169,7 @@ function applyBindingsToNodeAndDescendantsInternal(
     //  * For children of a *virtual* element, we can't be sure. Evaluating .parentNode on those children may
     //    skip over any number of intermediate virtual elements, any of which might define a custom binding context,
     //    hence bindingContextsMayDifferFromDomParentElement is true
-    applyBindingsToDescendantsInternal(bindingContext, nodeVerified, asyncBindingsApplied)
+    applyBindingsToDescendantsInternal(bindingContextForDescendants, nodeVerified, asyncBindingsApplied)
   }
 }
 
@@ -276,6 +277,7 @@ function applyBindingsToNodeInternal<T>(
   }
 
   let bindingHandlerThatControlsDescendantBindings: string | undefined
+  let contextToExtend = bindingContext
   if (bindings) {
     const $component = bindingContext.$component || {}
 
@@ -324,8 +326,27 @@ function applyBindingsToNodeInternal<T>(
       )
     }
 
+    if (bindingEvent.descendantsComplete in bindings) {
+      // Register the node as (potentially) completing asynchronously and thread
+      // the resulting context to descendants so nested async content reports
+      // completion up to this node. The callback re-checks `firstChild` at fire
+      // time, so a conditional that renders nothing suppresses the callback yet
+      // still re-arms when it later renders content.
+      contextToExtend = bindingEvent.startPossiblyAsyncContentBinding(node, bindingContext)
+      bindingEvent.subscribe(
+        node,
+        bindingEvent.descendantsComplete,
+        () => {
+          const callback = evaluateValueAccessor(bindings![bindingEvent.descendantsComplete])
+          if (callback && virtualElements.firstChild(node)) {
+            callback(node)
+          }
+        },
+        null
+      )
+    }
+
     const bindingsGenerated = topologicalSortBindings(bindings, $component)
-    const nodeAsyncBindingPromises = new Set<Promise<any>>()
     for (const [key, BindingHandlerClass] of bindingsGenerated) {
       // Go through the sorted bindings, calling init and update for each
       const reportBindingError = function (during: string, errorCaptured: Error) {
@@ -351,7 +372,7 @@ function applyBindingsToNodeInternal<T>(
             new BindingHandlerClass({
               allBindings,
               $element: node,
-              $context: bindingContext,
+              $context: contextToExtend,
               onError: reportBindingError,
               valueAccessor(...v) {
                 return getValueAccessor(key)(...v)
@@ -381,43 +402,18 @@ function applyBindingsToNodeInternal<T>(
 
         if (bindingHandler.bindingCompleted instanceof Promise) {
           asyncBindingsApplied!.add(bindingHandler.bindingCompleted)
-          nodeAsyncBindingPromises.add(bindingHandler.bindingCompleted)
         }
       } catch (err) {
         const error = err instanceof Error ? err : new Error(String(err))
         reportBindingError('creation', error)
       }
     }
-
-    triggerDescendantsComplete(node, bindings, nodeAsyncBindingPromises)
   }
 
   const shouldBindDescendants = bindingHandlerThatControlsDescendantBindings === undefined
-  return { shouldBindDescendants }
-}
-
-/**
- *
- * @param {HTMLElement} node
- * @param {Object} bindings
- * @param {[Promise]} nodeAsyncBindingPromises
- */
-function triggerDescendantsComplete(node: Node, bindings: object, nodeAsyncBindingPromises: Set<Promise<any>>) {
-  /** descendantsComplete ought to be an instance of the descendantsComplete
-   *  binding handler. */
-  const hasBindingHandler = bindingEvent.descendantsComplete in bindings
-  const hasFirstChild = virtualElements.firstChild(node)
-  const accessor = hasBindingHandler && evaluateValueAccessor(bindings[bindingEvent.descendantsComplete])
-  const callback = () => {
-    bindingEvent.notify(node, bindingEvent.descendantsComplete)
-    if (accessor && hasFirstChild) {
-      accessor(node)
-    }
-  }
-  if (nodeAsyncBindingPromises.size) {
-    Promise.all(nodeAsyncBindingPromises).then(callback)
-  } else {
-    callback()
+  return {
+    shouldBindDescendants,
+    bindingContextForDescendants: shouldBindDescendants ? contextToExtend : bindingContext
   }
 }