Skip to content

Commit

Permalink
Destroy components when changed on re-render
Browse files Browse the repository at this point in the history
Most of the infrastructure around components assumes they are static
(e.g. <my-component> cannot become a <foo-component> after it is
rendered). As such, there were some holes in destruction for the one
dynamic case (the {{component}} helper).

This commit detects when the underlying component has changed and
ensures that the component is scheduled for destruction before the new
component is swapped in.

It also fixes a similar issue with the view helper.
  • Loading branch information
tomdale committed Jun 11, 2015
1 parent ac8f0f6 commit ac846c3
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 6 deletions.
4 changes: 4 additions & 0 deletions packages/ember-htmlbars/lib/keywords/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ export default {
},

render(morph, ...rest) {
if (morph.state.manager) {
morph.state.manager.destroy();
}

// Force the component hook to treat this as a first-time render,
// because normal components (`<foo-bar>`) cannot change at runtime,
// but the `{{component}}` helper can.
Expand Down
5 changes: 5 additions & 0 deletions packages/ember-htmlbars/lib/keywords/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ export default {
options.createOptions._targetObject = node.state.targetObject;
}

if (state.manager) {
state.manager.destroy();
state.manager = null;
}

var nodeManager = ViewNodeManager.create(node, env, hash, options, parentView, null, scope, template);
state.manager = nodeManager;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,15 @@ ComponentNodeManager.prototype.rerender = function(_env, attrs, visitor) {
}, this);
};

ComponentNodeManager.prototype.destroy = function() {
let component = this.component;

// Clear component's render node. Normally this gets cleared
// during view destruction, but in this case we're re-assigning the
// node to a different view and it will get cleaned up automatically.
component._renderNode = null;
component.destroy();
};

export function createComponent(_component, isAngleBracket, _props, renderNode, env, attrs = {}) {
let props = assign({}, _props);
Expand Down Expand Up @@ -298,9 +307,6 @@ export function createComponent(_component, isAngleBracket, _props, renderNode,
}

component._renderNode = renderNode;
if (renderNode.emberView && renderNode.emberView !== component) {
throw new Error('Need to clean up this view before blindly reassigning.');
}
renderNode.emberView = component;
return component;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ ViewNodeManager.prototype.rerender = function(env, attrs, visitor) {
}, this);
};

ViewNodeManager.prototype.destroy = function() {
this.component.destroy();
};

function getTemplate(componentOrView) {
return componentOrView.isComponent ? get(componentOrView, '_template') : get(componentOrView, 'template');
}
Expand Down Expand Up @@ -192,9 +196,6 @@ export function createOrUpdateComponent(component, options, createOptions, rende

component._renderNode = renderNode;

if (renderNode.emberView && renderNode.emberView !== component) {
throw new Error('Need to clean up this view before blindly reassigning.');
}
renderNode.emberView = component;
return component;
}
Expand Down

0 comments on commit ac846c3

Please sign in to comment.