tko.bind) Fix issue with the dependency chain and huge performance pr…

…oblem

This addresses the on-again/off-again performance problem experienced and noted here: https://stackoverflow.com/questions/44414554

The synchronous `applyBindings` that was being called in various bindings (e.g. `template`, `if`, `with`) was being repeated for all child bindings, causing large refreshes.

The problem was caused when those bindings (`template`, `if`, etc) were changed to be descendants of `BindingHandler`, and were no longer converted via the `LegacyBindingHandler`.  What was the `init` function of those bindings became the `constructor` method of the new handler, but with the important difference that the `constructor` was not being called inside an `dependencyDetection.ignore`.

karma.conf.js

@@ -12,7 +12,7 @@ const pkg = JSON.parse(fs.readFileSync('package.json'))
 
 const root = path.join(process.cwd(), 'spec')
 
-console.log(` 🏕  Karma being loaded at ${process.cwd()}.`)
+console.log(` 🏕  Karma being loaded at ${process.cwd()}`)
 
 if (!pkg.karma || !pkg.karma.frameworks) {
   console.warn(`

packages/tko.bind/src/LegacyBindingHandler.js

@@ -32,12 +32,8 @@ export class LegacyBindingHandler extends BindingHandler {
     }
 
     try {
-      dependencyDetection.ignore(() => {
-        this.initReturn = handler.init && handler.init(...this.legacyArgs)
-      })
-    } catch (e) {
-      params.onError('init', e)
-    }
+      this.initReturn = handler.init && handler.init(...this.legacyArgs)
+    } catch (e) { params.onError('init', e) }
 
     if (typeof handler.update === 'function') {
       this.computed(() => {

packages/tko.bind/src/applyBindings.js

@@ -267,13 +267,15 @@ function applyBindingsToNodeInternal (node, sourceBindings, bindingContext, bind
       }
 
       try {
-        const bindingHandler = new BindingHandlerClass({
-          allBindings,
-          $element: node,
-          $context: bindingContext,
-          onError: reportBindingError,
-          valueAccessor (...v) { return getValueAccessor(key)(...v) }
-        })
+        const bindingHandler = dependencyDetection.ignore(() =>
+          new BindingHandlerClass({
+            allBindings,
+            $element: node,
+            $context: bindingContext,
+            onError: reportBindingError,
+            valueAccessor (...v) { return getValueAccessor(key)(...v) }
+          })
+        )
 
               // Expose the bindings via domData.
         allBindingHandlers[key] = bindingHandler

packages/tko.binding.foreach/src/foreach.js

@@ -73,15 +73,15 @@ function extend$context (include$index, $context) {
 }
 
 export class ForEachBinding extends AsyncBindingHandler {
-  // Valid valueAccessors:
+  // NOTE: valid valueAccessors include:
   //    []
   //    observable([])
   //    observableArray([])
   //    computed
   //    {data: array, name: string, as: string}
+
   constructor (params) {
     super(params)
-
     const settings = {}
     if (isPlainObject(this.value)) {
       Object.assign(settings, this.value)
@@ -123,7 +123,7 @@ export class ForEachBinding extends AsyncBindingHandler {
     virtualElements.emptyNode(this.$element)
 
     // Prime content
-    var primeData = unwrap(this.data)
+    const primeData = unwrap(this.data)
     if (primeData.map) {
       this.onArrayChange(primeData.map(valueToChangeAddItem), true)
     } else {

packages/tko.binding.if/src/ifIfnotWith.js

@@ -52,10 +52,7 @@ function cloneIfElseNodes (element, hasElse) {
     }
   }
 
-  return {
-    ifNodes: ifNodes,
-    elseNodes: elseNodes
-  }
+  return { ifNodes, elseNodes }
 }
 
 /**
@@ -150,6 +147,11 @@ export class ElseBindingHandler extends ConditionalBindingHandler {
    * @return {object}      { elseChainSatisfied: observable }
    */
   get elseChainIsAlreadySatisfied () {
+    if (!this._elseChain) { this._elseChain = this.readElseChain() }
+    return unwrap(this._elseChain.elseChainSatisfied)
+  }
+
+  readElseChain () {
     let node = this.$element
     do {
       node = node.previousSibling
@@ -161,8 +163,7 @@ export class ElseBindingHandler extends ConditionalBindingHandler {
       node = virtualElements.previousSibling(node)
     }
 
-    const conditionObject = domData.get(node, 'conditional') || {}
-    return unwrap(conditionObject.elseChainSatisfied)
+    return domData.get(node, 'conditional') || {}
   }
 }
 

packages/tko.binding.template/spec/nativeTemplateEngineBehaviors.js

@@ -3,7 +3,7 @@ import {
 } from 'tko.bind';
 
 import {
-    observableArray
+    observable, observableArray
 } from 'tko.observable';
 
 import {
@@ -189,6 +189,34 @@ describe('Native template engine', function() {
             expect(testNode.childNodes[0].childNodes[0]).toContainText("(Val: A1, Invocations: 1, Parents: 2)(Val: A2, Invocations: 2, Parents: 2)(Val: A3, Invocations: 3, Parents: 2)");
             expect(testNode.childNodes[0].childNodes[1]).toContainText("(Val: ANew, Invocations: 6, Parents: 2)(Val: B1, Invocations: 4, Parents: 2)(Val: B2, Invocations: 5, Parents: 2)");
         });
+
+        it('re-renders nested templates', function () {
+            var node = document.createElement('div')
+            let inner = 0
+            let outer = 0
+            node.innerHTML = `
+              <div data-bind='template: { data: x }'>
+                <i data-bind='incrementInner'></i>
+                <div data-bind='template: { data: $parent.y }'>
+                  <i data-bind='incrementOuter'></i>
+                </div>
+              </div>
+            `
+            options.bindingProviderInstance.bindingHandlers.set('incrementOuter', () => outer++)
+            options.bindingProviderInstance.bindingHandlers.set('incrementInner', () => inner++)
+            const x = observable()
+            const y = observable()
+            applyBindings({ x, y }, node)
+            expect(inner).toEqual(1)
+            expect(outer).toEqual(1)
+            x('a')
+            expect(inner).toEqual(2)
+            expect(outer).toEqual(2)
+            y('a')
+            expect(inner).toEqual(2)
+            expect(outer).toEqual(3)
+        })
+
     });
 
     describe('Data-bind syntax', function() {

packages/tko.binding.template/src/templating.js

@@ -131,7 +131,7 @@ function executeTemplate (targetNodeOrNodeArray, renderMode, template, bindingCo
 
   if (haveAddedNodesToParent) {
     activateBindingsOnContinuousNodeArray(renderedNodesArray, bindingContext, afterBindingCallback)
-    if (options['afterRender']) { dependencyDetection.ignore(options['afterRender'], null, [renderedNodesArray, bindingContext['$data']]) }
+    if (options.afterRender) { dependencyDetection.ignore(options.afterRender, null, [renderedNodesArray, bindingContext['$data']]) }
   }
 
   return renderedNodesArray
@@ -140,13 +140,13 @@ function executeTemplate (targetNodeOrNodeArray, renderMode, template, bindingCo
 function resolveTemplateName (template, data, context) {
     // The template can be specified as:
   if (isObservable(template)) {
-        // 1. An observable, with string value
+    // 1. An observable, with string value
     return template()
   } else if (typeof template === 'function') {
-        // 2. A function of (data, context) returning a string
+    // 2. A function of (data, context) returning a string
     return template(data, context)
   } else {
-        // 3. A string
+    // 3. A string
     return template
   }
 }
@@ -164,7 +164,7 @@ export function renderTemplate (template, dataOrBindingContext, options, targetN
 
     return computed( // So the DOM is automatically updated when any dependency changes
       function () {
-          // Ensure we've got a proper binding context to work with
+        // Ensure we've got a proper binding context to work with
         var bindingContext = (dataOrBindingContext && (dataOrBindingContext instanceof BindingContextConstructor))
               ? dataOrBindingContext
               : new BindingContextConstructor(dataOrBindingContext, null, null, null, { 'exportDependencies': true })
@@ -229,17 +229,11 @@ export default function renderTemplateForEach (template, arrayOrObservableArray,
   }, null, { disposeWhenNodeIsRemoved: targetNode })
 }
 
-var templateComputedDomDataKey = domData.nextKey()
-function disposeOldComputedAndStoreNewOne (element, newComputed) {
-  var oldComputed = domData.get(element, templateComputedDomDataKey)
-  if (oldComputed && (typeof (oldComputed.dispose) === 'function')) { oldComputed.dispose() }
-  domData.set(element, templateComputedDomDataKey, (newComputed && newComputed.isActive()) ? newComputed : undefined)
-}
+let templateComputedDomDataKey = domData.nextKey()
 
 export class TemplateBindingHandler extends AsyncBindingHandler {
   constructor (params) {
     super(params)
-
     const element = this.$element
     const bindingValue = unwrap(this.value)
     let container
@@ -270,7 +264,6 @@ export class TemplateBindingHandler extends AsyncBindingHandler {
       container = moveCleanedNodesToContainerElement(templateNodes) // This also removes the nodes from their current parent
       new AnonymousTemplate(element).nodes(container)
     }
-
     this.computed(this.onValueChange.bind(this))
   }
 
@@ -319,7 +312,13 @@ export class TemplateBindingHandler extends AsyncBindingHandler {
     }
 
     // It only makes sense to have a single template computed per element (otherwise which one should have its output displayed?)
-    disposeOldComputedAndStoreNewOne(element, templateComputed)
+    this.disposeOldComputedAndStoreNewOne(element, templateComputed)
+  }
+
+  disposeOldComputedAndStoreNewOne (element, newComputed) {
+    let oldComputed = domData.get(element, templateComputedDomDataKey)
+    if (oldComputed && (typeof oldComputed.dispose === 'function')) { oldComputed.dispose() }
+    domData.set(element, templateComputedDomDataKey, (newComputed && newComputed.isActive()) ? newComputed : undefined)
   }
 
   get controlsDescendants () { return true }