Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Merge branch 'fix-ui-hooks-nested-domranges' into release-0.8.2

  • Loading branch information...
commit 7b1cddad0ca419a5000dc46aea2e677328ab0413 2 parents 631bce6 + 0854415
Emily Stark estark37 authored
14 packages/spacebars-tests/template_tests.html
View
@@ -779,6 +779,20 @@
</div>
</template>
+<template name="spacebars_test_ui_hooks_nested">
+ {{#if foo}}
+ {{> spacebars_test_ui_hooks_nested_sub}}
+ {{/if}}
+</template>
+
+<template name="spacebars_test_ui_hooks_nested_sub">
+ <div>
+ {{#with true}}
+ <p>hello</p>
+ {{/with}}
+ </div>
+</template>
+
<template name="spacebars_test_template_instance_helper">
{{#with true}}{{foo}}{{/with}}
</template>
34 packages/spacebars-tests/template_tests.js
View
@@ -2094,6 +2094,40 @@ Tinytest.add(
);
Tinytest.add(
+ "spacebars - ui hooks - nested domranges",
+ function (test) {
+ var tmpl = Template.spacebars_test_ui_hooks_nested;
+ var rv = new ReactiveVar(true);
+
+ tmpl.foo = function () {
+ return rv.get();
+ };
+
+ var subtmpl = Template.spacebars_test_ui_hooks_nested_sub;
+ var uiHookCalled = false;
+ subtmpl.rendered = function () {
+ this.firstNode.parentNode._uihooks = {
+ removeElement: function (node) {
+ uiHookCalled = true;
+ }
+ };
+ };
+
+ var div = renderToDiv(tmpl);
+ document.body.appendChild(div);
+ Deps.flush();
+
+ var htmlBeforeRemove = canonicalizeHtml(div.innerHTML);
+ rv.set(false);
+ Deps.flush();
+ test.isTrue(uiHookCalled);
+ var htmlAfterRemove = canonicalizeHtml(div.innerHTML);
+ test.equal(htmlBeforeRemove, htmlAfterRemove);
+ document.body.removeChild(div);
+ }
+);
+
+Tinytest.add(
"spacebars - access template instance from helper",
function (test) {
// Set a property on the template instance; check that it's still
10 packages/ui/dombackend.js
View
@@ -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] = [];
34 packages/ui/dombackend_tests.js
View
@@ -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
38 packages/ui/domrange.js
View
@@ -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]);
-
- if (! elementsAlreadyRemoved)
- DomBackend.removeElement(node);
+ rangeRemoved(comps[i], true /* alreadyTornDown */);
+
+ // `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;
}
7 packages/ui/domrange_tests.js
View
@@ -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);
2  packages/ui/render.js
View
@@ -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();
});
}
Please sign in to comment.
Something went wrong with that request. Please try again.