Skip to content

Commit

Permalink
Merge pull request #14987 from emberjs/mem-leak
Browse files Browse the repository at this point in the history
[BUGFIX release] don’t leak last destroyedComponents
  • Loading branch information
chadhietala authored Mar 8, 2017
2 parents 9d6b72f + 6b6fc4d commit dacfaa5
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 5 deletions.
10 changes: 5 additions & 5 deletions packages/ember-glimmer/lib/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export default class Environment extends GlimmerEnvironment {
this.isInteractive = owner.lookup('-environment:main').isInteractive;

// can be removed once https://github.com/tildeio/glimmer/pull/305 lands
this.destroyedComponents = undefined;
this.destroyedComponents = [];

installPlatformSpecificProtocolForURL(this);

Expand Down Expand Up @@ -275,16 +275,16 @@ export default class Environment extends GlimmerEnvironment {
this.inTransaction = true;

super.begin();

this.destroyedComponents = [];
}

commit() {
let destroyedComponents = this.destroyedComponents;
this.destroyedComponents = [];
// components queued for destruction must be destroyed before firing
// `didCreate` to prevent errors when removing and adding a component
// with the same name (would throw an error when added to view registry)
for (let i = 0; i < this.destroyedComponents.length; i++) {
this.destroyedComponents[i].destroy();
for (let i = 0; i < destroyedComponents.length; i++) {
destroyedComponents[i].destroy();
}

super.commit();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { set } from 'ember-metal';
import { Component } from '../../utils/helpers';
import { moduleFor, RenderingTest } from '../../utils/test-case';

moduleFor('Component destroy', class extends RenderingTest {
['@test it correctly releases the destroyed components'](assert) {
let FooBarComponent = Component.extend({});

this.registerComponent('foo-bar', { ComponentClass: FooBarComponent, template: 'hello' });

this.render('{{#if switch}}{{#foo-bar}}{{foo-bar}}{{/foo-bar}}{{/if}}', { switch: true });

this.assertComponentElement(this.firstChild, { content: 'hello' });

this.runTask(() => set(this.context, 'switch', false));

this.assertText('');

assert.equal(this.env.destroyedComponents.length, 0, 'enviroment.destroyedComponents should be empty');
}
});

0 comments on commit dacfaa5

Please sign in to comment.