Skip to content

Commit

Permalink
Merge pull request #12272 from rwjblue/update-htmlbrs
Browse files Browse the repository at this point in the history
Update HTMLBars to fix memory leak with {{each}} inside {{if}}.
  • Loading branch information
rwjblue committed Sep 2, 2015
2 parents 62830c1 + 92646d4 commit 6d66c6b
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 10 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"finalhandler": "^0.4.0",
"github": "^0.2.3",
"glob": "~4.3.2",
"htmlbars": "0.14.3",
"htmlbars": "0.14.4",
"qunit-extras": "^1.3.0",
"qunitjs": "^1.16.0",
"route-recognizer": "0.1.5",
Expand Down
4 changes: 2 additions & 2 deletions packages/ember-htmlbars/lib/hooks/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ export default function componentHook(renderNode, env, scope, _tagName, params,
let contentOptions = { templates, scope };

let { block } = buildComponentTemplate(templateOptions, attrs, contentOptions);
block(env, [], undefined, renderNode, scope, visitor);
block.invoke(env, [], undefined, renderNode, scope, visitor);
} else if (isNormalHTMLElement) {
let block = buildHTMLTemplate(tagName, attrs, { templates, scope });
block(env, [], undefined, renderNode, scope, visitor);
block.invoke(env, [], undefined, renderNode, scope, visitor);
} else {
// Invoking a component from the outside (either via <foo-bar> angle brackets
// or {{foo-bar}} legacy curlies).
Expand Down
4 changes: 2 additions & 2 deletions packages/ember-htmlbars/lib/keywords/legacy-yield.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ export default function legacyYield(morph, env, _scope, params, hash, template,
scope.locals.controller = new ProxyStream(hash.controller, 'controller');
scope.overrideController = true;
}
scope.blocks.default(env, [], params[0], morph, scope, visitor);
scope.blocks.default.invoke(env, [], params[0], morph, scope, visitor);
} else {
scope.blocks.default(env, params, undefined, morph, scope, visitor);
scope.blocks.default.invoke(env, params, undefined, morph, scope, visitor);
}

return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ ComponentNodeManager.prototype.render = function(_env, visitor) {
env.renderedViews.push(component.elementId);

if (this.block) {
this.block(env, [], undefined, this.renderNode, this.scope, visitor);
this.block.invoke(env, [], undefined, this.renderNode, this.scope, visitor);
}

let element;
Expand Down Expand Up @@ -232,7 +232,7 @@ ComponentNodeManager.prototype.rerender = function(_env, attrs, visitor) {
env.renderedViews.push(component.elementId);

if (this.block) {
this.block(env, [], undefined, this.renderNode, this.scope, visitor);
this.block.invoke(env, [], undefined, this.renderNode, this.scope, visitor);
}

env.lifecycleHooks.push({ type: 'didUpdate', view: component });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ ViewNodeManager.prototype.render = function(env, attrs, visitor) {
}

if (this.block) {
this.block(newEnv, [], undefined, this.renderNode, this.scope, visitor);
this.block.invoke(newEnv, [], undefined, this.renderNode, this.scope, visitor);
}

if (component) {
Expand Down Expand Up @@ -131,7 +131,7 @@ ViewNodeManager.prototype.rerender = function(env, attrs, visitor) {
env.renderedViews.push(component.elementId);
}
if (this.block) {
this.block(newEnv, [], undefined, this.renderNode, this.scope, visitor);
this.block.invoke(newEnv, [], undefined, this.renderNode, this.scope, visitor);
}

return newEnv;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ if (isEnabled('ember-htmlbars-component-generation')) {
component = this;
this._super(...arguments);
},
layout: compile(`<my-component>{{yield}}</my-component>`)
layout: compile(`<my-component>...{{yield}}...</my-component>`)
});

renderComponent('my-component', {
Expand All @@ -34,6 +34,7 @@ if (isEnabled('ember-htmlbars-component-generation')) {
ok(component instanceof GlimmerComponent, 'the component was instantiated correctly');
equal(view.childViews[0], component, 'the component was rendered and inserted into child views');
hasSelector(assert, `my-component.ember-view[id=${component.elementId}]`);
equal(view.$().text(), '...Hello world...');
});
}

Expand Down
44 changes: 44 additions & 0 deletions packages/ember-htmlbars/tests/helpers/if_unless_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,20 @@ import run from 'ember-metal/run_loop';
import Namespace from 'ember-runtime/system/namespace';
import { Registry } from 'ember-runtime/system/container';
import EmberView from 'ember-views/views/view';
import Component from 'ember-views/components/component';
import ObjectProxy from 'ember-runtime/system/object_proxy';
import EmberObject from 'ember-runtime/system/object';
import compile from 'ember-template-compiler/system/compile';
import ArrayProxy from 'ember-runtime/system/array_proxy';
import { A as emberA } from 'ember-runtime/system/native_array';

import { set } from 'ember-metal/property_set';
import { typeOf } from 'ember-runtime/utils';
import { runAppend, runDestroy } from 'ember-runtime/tests/utils';

import { registerKeyword, resetKeyword } from 'ember-htmlbars/tests/utils';
import viewKeyword from 'ember-htmlbars/keywords/view';
import ComponentLookup from 'ember-views/component_lookup';

var originalLookup = Ember.lookup;

Expand All @@ -28,7 +31,10 @@ QUnit.module('ember-htmlbars: {{#if}} and {{#unless}} helpers', {
registry = new Registry();
container = registry.container();
registry.optionsForType('template', { instantiate: false });
registry.optionsForType('view', { singleton: false });
registry.optionsForType('component', { singleton: false });
registry.register('view:toplevel', EmberView.extend());
registry.register('component-lookup:main', ComponentLookup);
},

teardown() {
Expand Down Expand Up @@ -897,3 +903,41 @@ QUnit.test('`if` helper with inline form: updates when given a falsey second arg

equal(view.$().text(), 'falsy');
});

QUnit.test('using `if` with an `{{each}}` destroys components when transitioning to and from inverse (GH #12267)', function() {
let destroyedChildrenCount = 0;

registry.register('component:foo-bar', Component.extend({
willDestroy() {
destroyedChildrenCount++;
}
}));
registry.register('template:components/foo-bar', compile('{{number}}'));

view = EmberView.create({
container,
test: true,
list: emberA([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]),

template: compile(`
{{~#if view.test~}}
{{~#each view.list as |number|~}}
{{~foo-bar number=number~}}
{{~/each~}}
{{~else~}}
Nothing Here!
{{~/if~}}`)
});

runAppend(view);

equal(view.$().text(), '12345678910');

run(() => {
view.set('test', false);
});

equal(view.$().text(), 'Nothing Here!');

equal(destroyedChildrenCount, 10, 'the children were properly destroyed');
});

0 comments on commit 6d66c6b

Please sign in to comment.