Merge pull request #1065 from glimmerjs/bugfix/fix-will-destroy-in-each

[BUGFIX] Ensure {{each}} updates register destructors
glimmerjs · Mar 24, 2020 · 73e2320 · 73e2320
2 parents 858de79 + 01aa342
commit 73e2320
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 2 deletions.
diff --git a/packages/@glimmer/integration-tests/test/updating-test.ts b/packages/@glimmer/integration-tests/test/updating-test.ts
@@ -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,
@@ -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
 }
 

diff --git a/packages/@glimmer/runtime/lib/vm/update.ts b/packages/@glimmer/runtime/lib/vm/update.ts
@@ -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>());
@@ -242,6 +242,8 @@ class ListRevalidationDelegate implements IteratorSynchronizerDelegate<Environme
 
     updating.insertBefore(tryOpcode!, reference);
 
+    associate(opcode, result.drop);
+
     this.didInsert = true;
   }