Skip to content

Commit

Permalink
fix(components) $dispose not always called
Browse files Browse the repository at this point in the history
Let's consider a template like that
```
<template test(data)>
  <#data.ref />
</template>
```

When changing the component template behinf `data.ref` corresponding
`$dispose()` method (if any defined in the controller class) were not
executed.

Closes ariatemplates#306
  • Loading branch information
Benoit Charbonnier committed Sep 24, 2014
1 parent 711d77d commit a4d9770
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 13 deletions.
33 changes: 20 additions & 13 deletions hsp/rt/$root.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ var CPT_TYPES={
var DOCUMENT_FRAGMENT_NODE = 11;

/**
* Root node - created at the root of each template
* Contains the listeners requested by the child nodes
* Root node - created at the root of each template
* Contains the listeners requested by the child nodes
* Is replaced by the $CptNode (child class) when the template is inserted in another template
*/
var $RootNode = klass({
Expand Down Expand Up @@ -411,7 +411,7 @@ var $CptNode = klass({
ni.node2=node2;

if (p.cptType==="$CptAttInsert") {
// this cpt is used to an insert another component passed as attribute
// this cpt is used to an insert another component passed as attribute
ni.initCpt(po);
} else {
// we are in a template or component cpt
Expand All @@ -425,20 +425,20 @@ var $CptNode = klass({
// create an element to avoid generating other errors
ni=this.createCptInstance("$CptAttInsert",parent);
}

return ni;
},

/**
* Calculates the object referenced by the path and the component type
* @return {Object} object with the following properties:
* pathObject: {Object} the object referenced by the path
* cptType: {String} one of the following option: "$CptComponent",
* cptType: {String} one of the following option: "$CptComponent",
* "$CptTemplate", "$CptAttInsert" or "InvalidComponent"
*/
getPathData:function(path, vscope) {
// determine the type of this component:
// - either a template - e.g. <#mytemplate foo="bar"/>
// determine the type of this component:
// - either a template - e.g. <#mytemplate foo="bar"/>
// -> instance will extend $CptTemplate
// - a component with controller - e.g. <#mycpt foo="bar"/>
// -> instance will extend $CptComponent
Expand Down Expand Up @@ -513,7 +513,7 @@ var $CptNode = klass({
nd.appendChild(this.node2);
}
},

/**
* Callback called when a controller attribute or a template attribute has changed
*/
Expand Down Expand Up @@ -600,11 +600,19 @@ var $CptNode = klass({
if (tplChanged) {
// check if component nature changed from template to component or opposite
this.template=tpl;
// Change might also be a different component ref, ie cpt1 -> cpt2
if (this.isCptNode && this.controller && this.controller.$dispose) {
// If we have a $dispose method, let's call it.
this.controller.$dispose();
this.controller = null;
this.ctlAttributes = null;
}

this.createChildNodeInstances();
} else {
if (this.refreshAttributes) {
this.refreshAttributes();
// for component and sub-templates the original vscope is substituted
// for component and sub-templates the original vscope is substituted
// to the one of the component- or sub-template
// so we need to revert to the parent scope to observe the correct objects
var vs=this.vscope;
Expand Down Expand Up @@ -668,7 +676,7 @@ var $CptNode = klass({
var sz=pos.length;

this._pathChgeCb = this.onPathChange.bind(this);

for (var i=0;sz>i;i++) {
json.observe(pos[i], this._pathChgeCb);
}
Expand Down Expand Up @@ -747,7 +755,7 @@ var $CptAttElement = klass({
isCptAttElement : true,

/**
* $CptAttElement generator
* $CptAttElement generator
*/
$constructor : function (name, exps, attcfg, ehcfg, children) {
this.name = name;
Expand Down Expand Up @@ -789,7 +797,7 @@ var $CptAttElement = klass({
if (p.ctlAttributes) {
attDef=p.ctlAttributes[this.name];
}

if (!eltDef && !attDef) {
// invalid elt
log.error(this.info+" Element not supported by its parent component");
Expand Down Expand Up @@ -846,4 +854,3 @@ cptComponent.setDependency("$CptAttElement",$CptAttElement);
exports.$RootNode = $RootNode;
exports.$CptNode = $CptNode;
exports.$CptAttElement = $CptAttElement;

98 changes: 98 additions & 0 deletions test/rt/cptlifecyle.spec.hsp
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
* Copyright 2013 Amadeus s.a.s.
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

var klass=require("hsp/klass"),
ht=require("hsp/utils/hashtester");

var constructor_counter = 0;
var dispose_counter = 0;

var LifeCycleController = klass({
$constructor: function() {
constructor_counter++;
},
$dispose: function() {
dispose_counter++;
}
});

<template lifecycle1 using ctrl:LifeCycleController>
<div class="content">I am component 1</div>
</template>

<template lifecycle2 using ctrl:LifeCycleController>
<div class="content">I am component 2</div>
</template>

<template wrapper(data)>
{if data.visible}
<#lifecycle1 />
{/if}
</template>

<template dynamicWrapper(data)>
<#data.cpt />
</template>

var data = {
visible: true,
cpt: lifecycle1
};


describe("Component lifecycle", function () {
beforeEach(function() {
constructor_counter = dispose_counter = 0;
});
it("should invoke $contructor and $dispose anytime a new instance is needed", function() {
var h=ht.newTestContext();

wrapper(data).render(h.container);
expect(h(".content").text()).to.equal('I am component 1');
expect(constructor_counter).to.equal(1);
h.$set(data, "visible", false);
h.$set(data, "visible", true);
expect(constructor_counter).to.equal(2);

expect(dispose_counter).to.equal(1);

h.$dispose();
});
});

describe.only("Component used as dynamic <#ref >", function() {
beforeEach(function() {
constructor_counter = dispose_counter = 0;
});

it("should invoke $constructor at instance creation", function() {
var h=ht.newTestContext();

dynamicWrapper(data).render(h.container);
expect(h(".content").text()).to.equal('I am component 1');
expect(constructor_counter).to.equal(1);

h.$set(data, "cpt", lifecycle2);
expect(h(".content").text()).to.equal('I am component 2');
expect(constructor_counter).to.equal(2);

h.$set(data, "cpt", lifecycle1);

expect(dispose_counter).to.equal(2);

h.$dispose();
});

});

0 comments on commit a4d9770

Please sign in to comment.