Separate element removal and “teardown”

DomRange now never removes elements except through the removeElement UI hook.  If you write a hook that prevents removal, teardown still happens (e.g. templates stop updating).

This code also provides the basis for stopping updates to part of the DOM by triggering teardown without removal.

Before, DomBackend.removeElement would both trigger teardown and actually deparent the element.  Now we have DomBackend.tearDownElement, which just triggers the jQuery teardown (which in turn triggers finalization of DomRanges that have been inserted in the DOM tree).

The flag to {node,members,range}Removed is now named “alreadyTornDown” and documented.  Its purpose is to prevent redundant teardown walks through the DOM.

packages/ui/dombackend.js

@@ -28,7 +28,7 @@ if (Meteor.isClient) {
 
   // Causes `elem` (a DOM element) to be detached from its parent, if any.
   // Whether or not `elem` was detached, causes any callbacks registered
-  // with `onRemoveElement` on `elem` and its descendants to fire.
+  // with `onElementTeardown` on `elem` and its descendants to fire.
   // Not for use on non-element nodes.
   //
   // This method is modeled after the behavior of jQuery's `$(elem).remove()`,
@@ -37,11 +37,17 @@ if (Meteor.isClient) {
     $jq(elem).remove();
   };
 
+  DomBackend.tearDownElement = function (elem) {
+    var elems = Array.prototype.slice.call(elem.getElementsByTagName('*'));
+    elems.push(elem);
+    $jq.cleanData(elems);
+  };
+
   // Registers a callback function to be called when the given element or
   // one of its ancestors is removed from the DOM via the backend library.
   // The callback function is called at most once, and it receives the element
   // in question as an argument.
-  DomBackend.onRemoveElement = function (elem, func) {
+  DomBackend.onElementTeardown = function (elem, func) {
     if (! elem[REMOVAL_CALLBACKS_PROPERTY_NAME]) {
       elem[REMOVAL_CALLBACKS_PROPERTY_NAME] = [];
 

packages/ui/dombackend_tests.js

@@ -35,16 +35,16 @@ var isDetachedSingleNode = function (test, node) {
 };
 
 Tinytest.add("ui - DomBackend - element removal", function (test) {
-  // Test that calling removeElement on a detached element calls onRemoveElement
+  // Test that calling removeElement on a detached element calls onElementTeardown
   // on it and its descendents. For jQuery, `removeElement` runs `$(elem).remove()`,
   // so it tests detecting a jQuery removal, as well as the stronger condition
   // that clean-up still happens on the DOM tree in the detached case.
   runDivSpanBTest(function (div, span, b, buf, func1, func2, func3, func4) {
-    DomBackend.onRemoveElement(div, func1);
-    DomBackend.onRemoveElement(span, func2);
-    DomBackend.onRemoveElement(b, func3);
+    DomBackend.onElementTeardown(div, func1);
+    DomBackend.onElementTeardown(span, func2);
+    DomBackend.onElementTeardown(b, func3);
     // test second callback on same element
-    DomBackend.onRemoveElement(div, func4);
+    DomBackend.onElementTeardown(div, func4);
 
     DomBackend.removeElement(div); // "remove" the (parentless) DIV
 
@@ -59,10 +59,10 @@ Tinytest.add("ui - DomBackend - element removal", function (test) {
   // Test that `removeElement` actually removes the element
   // (and fires appropriate callbacks).
   runDivSpanBTest(function (div, span, b, buf, func1, func2, func3, func4) {
-    DomBackend.onRemoveElement(div, func1);
-    DomBackend.onRemoveElement(span, func2);
-    DomBackend.onRemoveElement(b, func3);
-    DomBackend.onRemoveElement(div, func4);
+    DomBackend.onElementTeardown(div, func1);
+    DomBackend.onElementTeardown(span, func2);
+    DomBackend.onElementTeardown(b, func3);
+    DomBackend.onElementTeardown(div, func4);
 
     DomBackend.removeElement(span); // remove the SPAN
 
@@ -83,10 +83,10 @@ Tinytest.add("ui - DomBackend - element removal (jQuery)", function (test) {
 
   // Test with `$(elem).remove()`.
   runDivSpanBTest(function (div, span, b, buf, func1, func2, func3, func4) {
-    DomBackend.onRemoveElement(div, func1);
-    DomBackend.onRemoveElement(span, func2);
-    DomBackend.onRemoveElement(b, func3);
-    DomBackend.onRemoveElement(div, func4);
+    DomBackend.onElementTeardown(div, func1);
+    DomBackend.onElementTeardown(span, func2);
+    DomBackend.onElementTeardown(b, func3);
+    DomBackend.onElementTeardown(div, func4);
 
     $(span).remove(); // remove the SPAN
 
@@ -103,10 +103,10 @@ Tinytest.add("ui - DomBackend - element removal (jQuery)", function (test) {
 
   // Test that `$(elem).detach()` is NOT considered a removal.
   runDivSpanBTest(function (div, span, b, buf, func1, func2, func3, func4) {
-    DomBackend.onRemoveElement(div, func1);
-    DomBackend.onRemoveElement(span, func2);
-    DomBackend.onRemoveElement(b, func3);
-    DomBackend.onRemoveElement(div, func4);
+    DomBackend.onElementTeardown(div, func1);
+    DomBackend.onElementTeardown(span, func2);
+    DomBackend.onElementTeardown(b, func3);
+    DomBackend.onElementTeardown(div, func4);
 
     $(span).detach(); // detach the SPAN
 

packages/ui/domrange.js

@@ -11,7 +11,7 @@ var removeNode = function (n) {
       n.parentNode._uihooks && n.parentNode._uihooks.removeElement) {
     n.parentNode._uihooks.removeElement(n);
   } else {
-    DomBackend.removeElement(n);
+    n.parentNode.removeChild(n);
   }
 };
 
@@ -110,8 +110,8 @@ var rangeParented = function (range) {
       range._rangeDict = rangeDict;
 
       // get jQuery to tell us when this node is removed
-      DomBackend.onRemoveElement(parentNode, function () {
-        rangeRemoved(range, true /* elementsAlreadyRemoved */);
+      DomBackend.onElementTeardown(parentNode, function () {
+        rangeRemoved(range, true /* alreadyTornDown */);
       });
     }
 
@@ -128,7 +128,7 @@ var rangeParented = function (range) {
   }
 };
 
-var rangeRemoved = function (range, elementsAlreadyRemoved) {
+var rangeRemoved = function (range, alreadyTornDown) {
   if (! range.isRemoved) {
     range.isRemoved = true;
 
@@ -146,29 +146,39 @@ var rangeRemoved = function (range, elementsAlreadyRemoved) {
     if (range.removed)
       range.removed();
 
-    membersRemoved(range, elementsAlreadyRemoved);
+    membersRemoved(range, alreadyTornDown);
   }
 };
 
-var nodeRemoved = function (node, elementsAlreadyRemoved) {
+var nodeRemoved = function (node, alreadyTornDown) {
   if (node.nodeType === 1) { // ELEMENT
     var comps = DomRange.getComponents(node);
     for (var i = 0, N = comps.length; i < N; i++)
-      rangeRemoved(comps[i], elementsAlreadyRemoved);
+      rangeRemoved(comps[i], true /* alreadyTornDown */);
 
-    if (! elementsAlreadyRemoved)
-      DomBackend.removeElement(node);
+    // `alreadyTornDown` is an optimization so that we don't
+    // tear down the same elements multiple times when tearing
+    // down a tree of DomRanges and elements, leading to asymptotic
+    // inefficiency.
+    //
+    // When jQuery removes an element or DomBackend.tearDownElement
+    // is called, the DOM is "cleaned" recursively, calling all
+    // onElementTearDown handlers on the entire DOM subtree.
+    // Since the entire subtree is already walked, we don't want to
+    // also walk the subtrees of each DomRange for teardown purposes.
+    if (! alreadyTornDown)
+      DomBackend.tearDownElement(node);
   }
 };
 
-var membersRemoved = function (range, elementsAlreadyRemoved) {
+var membersRemoved = function (range, alreadyTornDown) {
   var members = range.members;
   for (var k in members) {
     var mem = members[k];
     if (mem instanceof DomRange)
-      rangeRemoved(mem, elementsAlreadyRemoved);
+      rangeRemoved(mem, alreadyTornDown);
     else
-      nodeRemoved(mem, elementsAlreadyRemoved);
+      nodeRemoved(mem, alreadyTornDown);
   }
 };
 
@@ -230,7 +240,7 @@ _extend(DomRange.prototype, {
     for (var i = 0, N = nodes.length; i < N; i++)
       removeNode(nodes[i]);
 
-    membersRemoved(this, true /* elementsAlreadyRemoved */);
+    membersRemoved(this);
 
     this.members = {};
   },
@@ -341,7 +351,7 @@ _extend(DomRange.prototype, {
       removeNode(this.start);
       removeNode(this.end);
       this.owner = null;
-      rangeRemoved(this, true /* elementsAlreadyRemoved */);
+      rangeRemoved(this);
       return;
     }
 

packages/ui/domrange_tests.js

@@ -25,7 +25,8 @@ var inDocument = function (range, func) {
   try {
     func(range);
   } finally {
-    document.body.removeChild(onscreen);
+    if (onscreen.parentNode === document.body)
+      document.body.removeChild(onscreen);
   }
 };
 
@@ -862,7 +863,7 @@ Tinytest.add("ui - DomRange - structural removal", function (test) {
     test.isTrue(e.isRemoved);
 
 
-    for (var scenario = 0; scenario < 2; scenario++) {
+    for (var scenario = 0; scenario < 3; scenario++) {
       var f = new DomRange;
       var g = document.createElement("DIV");
       var h = new DomRange;
@@ -882,6 +883,8 @@ Tinytest.add("ui - DomRange - structural removal", function (test) {
         r.removeAll();
       else if (scenario === 1)
         r.remove('f');
+      else if (scenario === 2)
+        $(r.parentNode()).remove();
       test.isTrue(f.isRemoved);
       test.isTrue(h.isRemoved);
       test.isTrue(k.isRemoved);

packages/ui/render.js

@@ -382,7 +382,7 @@ var materialize = function (node, parent, before, parentComponent) {
           reportUIException(e);
         }
       });
-      UI.DomBackend.onRemoveElement(elem, function () {
+      UI.DomBackend.onElementTeardown(elem, function () {
         attrComp.stop();
       });
     }