Merge pull request #1100 from glimmerjs/bugfix/iterable-item-only-upd…

…ate-on-change

[BUGFIX] Update iterable items only when changed

packages/@glimmer/integration-tests/lib/suites/each.ts

@@ -1,5 +1,6 @@
 import { RenderTest } from '../render-test';
 import { test } from '../test-decorator';
+import { tracked } from '../test-helpers/tracked';
 
 export class EachSuite extends RenderTest {
   static suiteName = '#each';
@@ -165,6 +166,38 @@ export class EachSuite extends RenderTest {
   'it renders all items with duplicate key values'() {
     let list = [{ text: 'Hello' }, { text: 'Hello' }, { text: 'Hello' }];
 
+    this.render(`{{#each list key="text" as |item|}}{{item.text}}{{/each}}`, {
+      list,
+    });
+
+    this.assertHTML('HelloHelloHello');
+    this.assertStableRerender();
+
+    list.forEach(item => (item.text = 'Goodbye'));
+
+    this.rerender({ list });
+    this.assertHTML('GoodbyeGoodbyeGoodbye');
+    this.assertStableNodes();
+
+    list = [{ text: 'Hello' }, { text: 'Hello' }, { text: 'Hello' }];
+
+    this.rerender({ list });
+    this.assertHTML('HelloHelloHello');
+    this.assertStableNodes();
+  }
+
+  @test
+  'it updates items if their key has not changed, and the items are tracked'() {
+    class Item {
+      @tracked public text: string;
+
+      constructor(text: string) {
+        this.text = text;
+      }
+    }
+
+    let list = [new Item('Hello'), new Item('Hello'), new Item('Hello')];
+
     this.render(`{{#each list key="@identity" as |item|}}{{item.text}}{{/each}}`, {
       list,
     });
@@ -185,6 +218,24 @@ export class EachSuite extends RenderTest {
     this.assertStableNodes();
   }
 
+  @test
+  'it does not update items if their key has not changed, and the items are not tracked'() {
+    let list = [{ text: 'Hello' }, { text: 'Hello' }, { text: 'Hello' }];
+
+    this.render(`{{#each list key="@identity" as |item|}}{{item.text}}{{/each}}`, {
+      list,
+    });
+
+    this.assertHTML('HelloHelloHello');
+    this.assertStableRerender();
+
+    list.forEach(item => (item.text = 'Goodbye'));
+
+    this.rerender({ list });
+    this.assertHTML('HelloHelloHello');
+    this.assertStableNodes();
+  }
+
   @test
   'scoped variable not available outside list'() {
     let list = ['Wycats'];
@@ -243,6 +294,14 @@ export class EachSuite extends RenderTest {
   }
 }
 
-function val(i: number): { val: number } {
-  return { val: i };
+class Val {
+  @tracked val: number;
+
+  constructor(val: number) {
+    this.val = val;
+  }
+}
+
+function val(i: number): Val {
+  return new Val(i);
 }

packages/@glimmer/integration-tests/lib/suites/in-element.ts

@@ -6,6 +6,7 @@ import { equalsElement } from '../dom/assertions';
 import { stripTight } from '../test-helpers/strings';
 import { replaceHTML } from '../dom/simple-utils';
 import { EmberishCurlyComponent } from '../components/emberish-curly';
+import { tracked } from '../test-helpers/tracked';
 
 export class InElementSuite extends RenderTest {
   static suiteName = '#in-element';
@@ -388,20 +389,28 @@ export class InElementSuite extends RenderTest {
 
   @test
   'Inside a loop'() {
+    let { delegate } = this;
+
+    class Item {
+      element = delegate.createElement('div');
+
+      @tracked value: string;
+
+      constructor(value: string) {
+        this.value = value;
+      }
+    }
+
     this.testType = 'Dynamic';
     this.registerComponent('Basic', 'FooBar', '<p>{{@value}}</p>');
 
     this.registerHelper('log', ([item]) => console.log(item));
 
-    let roots = [
-      { id: 0, element: this.delegate.createElement('div'), value: 'foo' },
-      { id: 1, element: this.delegate.createElement('div'), value: 'bar' },
-      { id: 2, element: this.delegate.createElement('div'), value: 'baz' },
-    ];
+    let roots = [new Item('foo'), new Item('bar'), new Item('baz')];
 
     this.render(
       stripTight`
-        {{~#each this.roots key="id" as |root|~}}
+        {{~#each this.roots as |root|~}}
           {{~log root~}}
           {{~#in-element root.element ~}}
             <FooBar @value={{root.value}} />

packages/@glimmer/integration-tests/lib/test-helpers/tracked.ts

@@ -0,0 +1,15 @@
+import { trackedData } from '@glimmer/validator';
+
+export function tracked(target: object, key: string) {
+  let { getter, setter } = trackedData<any, any>(key);
+
+  Object.defineProperty(target, key, {
+    get() {
+      return getter(this);
+    },
+
+    set(value: unknown) {
+      setter(this, value);
+    },
+  });
+}

packages/@glimmer/reference/lib/template.ts

@@ -307,8 +307,13 @@ export class IterationItemReference<T = unknown> implements TemplatePathReferenc
   }
 
   update(value: T) {
-    dirtyTag(this.tag);
-    this.itemValue = value;
+    let { itemValue } = this;
+
+    // TODO: refactor this https://github.com/glimmerjs/glimmer-vm/issues/1101
+    if (value !== itemValue) {
+      dirtyTag(this.tag);
+      this.itemValue = value;
+    }
   }
 
   get(key: string): TemplatePathReference {