Skip to content

Commit

Permalink
Fix double didMount bug.
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael Aufreiter committed Sep 8, 2015
1 parent 7e473a3 commit e71a511
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 11 deletions.
18 changes: 14 additions & 4 deletions test/unit/ui/component.test.js
Expand Up @@ -10,6 +10,7 @@ var TestComponent = Component.extend({
didInitialize: function() {
// make some methods inspectable
this.didMount = sinon.spy(this, 'didMount');
this.willUnmount = sinon.spy(this, 'willUnmount');
this.shouldRerender = sinon.spy(this, 'shouldRerender');
this.render = sinon.spy(this, 'render');
}
Expand Down Expand Up @@ -157,9 +158,9 @@ QUnit.test("Do deep rerender when state has changed.", function(assert) {
comp.setState({ foo: 'baz' });
assert.equal(comp.shouldRerender.callCount, 1, "Component should have been asked whether to rerender.");
assert.equal(comp.render.callCount, 2, "Component should have been rerendered.");
});
});

QUnit.test("Only call didMount once.", function(assert) {
QUnit.test("Only call didMount once for childs and grandchilds when setProps is called during mounting process.", function(assert) {
var Child = TestComponent.extend({
render: function() {
if (this.props.loading) {
Expand All @@ -180,7 +181,16 @@ QUnit.test("Only call didMount once.", function(assert) {
this.refs.child.setProps({ loading: false });
}
});

var comp = Component.mount($$(Parent), $('#qunit-fixture'));
assert.equal(comp.refs.child.didMount.callCount, 1, "Child's didMount should have been called only once.");
assert.equal(comp.refs.child.refs.child.didMount.callCount, 1, "Grandchild's didMount should have been called only once.");
var childComp = comp.refs.child;
var grandChildComp = childComp.refs.child;
assert.equal(childComp.didMount.callCount, 1, "Child's didMount should have been called only once.");
assert.equal(grandChildComp.didMount.callCount, 1, "Grandchild's didMount should have been called only once.");

comp.empty();
assert.equal(childComp.willUnmount.callCount, 1, "Child's willUnmount should have been called once.");
assert.equal(grandChildComp.willUnmount.callCount, 1, "Grandchild's willUnmount should have been called once.");

// TODO: can/should we somehow dispose the references to childComp and grandChildComp now?
});
20 changes: 13 additions & 7 deletions ui/component.js
Expand Up @@ -4,7 +4,6 @@ var OO = require('../basics/oo');
var _ = require('../basics/helpers');

var __id__ = 0;
var _isDocumentElement;
var _isInDocument;
var VirtualTextNode;
var RawHtml;
Expand Down Expand Up @@ -209,7 +208,10 @@ Component.Prototype = function ComponentPrototype() {
* ```
*/
this.triggerDidMount = function() {
this.didMount();
if (!this.__mounted__) {
this.__mounted__ = true;
this.didMount();
}
_.each(this.children, function(child) {
child.triggerDidMount();
});
Expand Down Expand Up @@ -237,6 +239,7 @@ Component.Prototype = function ComponentPrototype() {
this.unmount = function() {
this.triggerWillUnmount();
this.$el.remove();
this.__mounted__ = false;
// TODO: do we need to remove this from parents children
// right now it feels like that it doesn't make a great difference
// because most often this method is called by the parent during rerendering
Expand Down Expand Up @@ -1123,14 +1126,17 @@ Component.$$.prepareChildren = function(children) {
}
};

_isDocumentElement = function(el) {
// Node.DOCUMENT_NODE = 9
return (el.nodeType === 9);
};
/**
* Checks whether a given element has been injected in the document already
*
* We traverse up the DOM until we find the document root element. We return true
* if we can find it.
*/

_isInDocument = function(el) {
while(el) {
if (_isDocumentElement(el)) {
// Node.DOCUMENT_NODE = 9;
if (el.nodeType === 9) {
return true;
}
el = el.parentNode;
Expand Down

0 comments on commit e71a511

Please sign in to comment.