Merge pull request #1080 from glimmerjs/bugfix/iterable-update-item-d…

…estructors
glimmerjs · Apr 29, 2020 · bec3f43 · bec3f43
2 parents d99294d + 09f0190
commit bec3f43
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 13 deletions.
diff --git a/packages/@glimmer/integration-tests/test/updating-test.ts b/packages/@glimmer/integration-tests/test/updating-test.ts
@@ -1863,7 +1863,7 @@ class UpdatingTest extends RenderTest {
   }
 
   @test
-  'The each helper items destroy correctly (new and updated items)'() {
+  '{{each}} items destroy correctly when destroying the whole list (new and updated items)'() {
     let destroyCount = 0;
 
     this.registerComponent(
@@ -1897,6 +1897,41 @@ class UpdatingTest extends RenderTest {
     assert.equal(destroyCount, 2, 'both list items were correctly destroyed');
   }
 
+  @test
+  '{{each}} items destroy correctly if they were added after initial render'() {
+    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: ['initial'] });
+    assert.equal(destroyCount, 1, 'new list item was correctly destroyed');
+  }
+
   // TODO: port https://github.com/emberjs/ember.js/pull/14082
 }
 

diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/lists.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/lists.ts
@@ -43,8 +43,8 @@ APPEND_OPCODES.add(Op.Iterate, (vm, { op1: breaks }) => {
   let item = check(stack.peek(), CheckInstanceof(ReferenceIterator)).next();
 
   if (item) {
-    let tryOpcode = vm.iterate(item.memo, item.value);
-    vm.enterItem(item.key, tryOpcode);
+    let tryOpcode = vm.enterItem(item.memo, item.value);
+    vm.registerItem(item.key, tryOpcode);
   } else {
     vm.goto(breaks);
   }

diff --git a/packages/@glimmer/runtime/lib/vm/append.ts b/packages/@glimmer/runtime/lib/vm/append.ts
@@ -110,8 +110,8 @@ export interface InternalVM<C extends JitOrAotBlock = JitOrAotBlock> {
 
   enterList(offset: number): void;
   exitList(): void;
-  iterate(memo: PathReference<unknown>, item: PathReference<unknown>): TryOpcode;
-  enterItem(key: unknown, opcode: TryOpcode): void;
+  enterItem(memo: PathReference<unknown>, item: PathReference<unknown>): TryOpcode;
+  registerItem(key: unknown, opcode: TryOpcode): void;
 
   pushRootScope(size: number): PartialScope<C>;
   pushChildScope(): void;
@@ -363,7 +363,7 @@ export default abstract class VM<C extends JitOrAotBlock> implements PublicVM, I
     this.didEnter(tryOpcode);
   }
 
-  iterate(
+  enterItem(
     memo: VersionedPathReference<unknown>,
     value: VersionedPathReference<unknown>
   ): TryOpcode {
@@ -378,12 +378,14 @@ export default abstract class VM<C extends JitOrAotBlock> implements PublicVM, I
     // this.ip = end + 4;
     // this.frames.push(ip);
 
-    return new TryOpcode(state, this.runtime, block, new LinkedList<UpdatingOpcode>());
+    let opcode = new TryOpcode(state, this.runtime, block, new LinkedList<UpdatingOpcode>());
+    this.didEnter(opcode);
+
+    return opcode;
   }
 
-  enterItem(key: string, opcode: TryOpcode) {
+  registerItem(key: string, opcode: TryOpcode) {
     this.listBlock().map.set(key, opcode);
-    this.didEnter(opcode);
   }
 
   enterList(offset: number) {

diff --git a/packages/@glimmer/runtime/lib/vm/update.ts b/packages/@glimmer/runtime/lib/vm/update.ts
@@ -233,11 +233,9 @@ class ListRevalidationDelegate implements IteratorSynchronizerDelegate<Environme
     let tryOpcode: Option<TryOpcode> = null;
 
     let result = vm.execute(vm => {
-      tryOpcode = vm.iterate(memo, item);
+      vm.pushUpdating();
+      tryOpcode = vm.enterItem(memo, item);
       map.set(key, tryOpcode);
-      vm.pushUpdating(new LinkedList<UpdatingOpcode>());
-      vm.updateWith(tryOpcode);
-      vm.pushUpdating(tryOpcode.children);
     });
 
     updating.insertBefore(tryOpcode!, reference);