Skip to content

Commit

Permalink
[BUGFIX] Ensure {{each}} updates register destructors
Browse files Browse the repository at this point in the history
Destructors were not being registered for items added to each loops in
updates. This PR fixes this and adds a test.
  • Loading branch information
Chris Garrett committed Mar 24, 2020
1 parent 858de79 commit 01aa342
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 2 deletions.
37 changes: 36 additions & 1 deletion packages/@glimmer/integration-tests/test/updating-test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Option, HandleResult, ErrHandle, EncoderError } from '@glimmer/interfaces';
import { ConstReference } from '@glimmer/reference';
import { RenderTest, test, jitSuite, JitRenderDelegate } from '..';
import { RenderTest, test, jitSuite, JitRenderDelegate, EmberishGlimmerComponent } from '..';
import { PrimitiveReference, SafeString } from '@glimmer/runtime';
import {
assertNodeTagName,
Expand Down Expand Up @@ -1862,6 +1862,41 @@ class UpdatingTest extends RenderTest {
this.assertHTML('<ul><!----></ul>', 'After removing the remaining entries');
}

@test
'The each helper items destroy correctly (new and updated items)'() {
let destroyCount = 0;

this.registerComponent(
'Glimmer',
'DestroyableComponent',
'{{@item}}',
class extends EmberishGlimmerComponent {
destroy() {
destroyCount++;
}
}
);

this.render(
stripTight`
{{#each this.list as |item|}}
<div><DestroyableComponent @item={{item}}/></div>
{{/each}}
`,
{
list: ['initial'],
}
);

this.assertHTML(`<div>initial</div>`);

this.rerender({ list: ['initial', 'update'] });
this.assertHTML(`<div>initial</div><div>update</div>`);

this.rerender({ list: [] });
assert.equal(destroyCount, 2, 'both list items were correctly destroyed');
}

// TODO: port https://github.com/emberjs/ember.js/pull/14082
}

Expand Down
4 changes: 3 additions & 1 deletion packages/@glimmer/runtime/lib/vm/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ class ListRevalidationDelegate implements IteratorSynchronizerDelegate<Environme
let vm = opcode.vmForInsertion(nextSibling);
let tryOpcode: Option<TryOpcode> = null;

vm.execute(vm => {
let result = vm.execute(vm => {
tryOpcode = vm.iterate(memo, item);
map.set(key, tryOpcode);
vm.pushUpdating(new LinkedList<UpdatingOpcode>());
Expand All @@ -242,6 +242,8 @@ class ListRevalidationDelegate implements IteratorSynchronizerDelegate<Environme

updating.insertBefore(tryOpcode!, reference);

associate(opcode, result.drop);

this.didInsert = true;
}

Expand Down

0 comments on commit 01aa342

Please sign in to comment.