[BUGFIX] Fix reactivity of dynamic attributes

The updates to use autotracking through the VM resulted in a bug in
dynamic attributes. Previously, the updating opcode for dynamic
attributes would run its update function based on tags/autotracking -
whenever any tag updated (e.g. a tracked value was changed) the
attribute would call its `update` function. The `update` function would
then check the new value against the current value of the attribute, and
update if they were different.

The bug was we introduced a value comparison in the updating opcode
itself, based on the last value of the property to set the attribute to.
This meant that if the property was updated to the same value twice in
a row, it would not call `update`, even if the attribute's value had
changed (e.g. because the user typed something).

Author: Chris Garrett

packages/@glimmer/integration-tests/test/attributes-test.ts

@@ -1,6 +1,7 @@
 import { normalizeProperty } from '@glimmer/runtime';
-import { assertElement, hasAttribute, jitSuite, RenderTest, test } from '..';
+import { assertElement, hasAttribute, jitSuite, RenderTest, test, tracked } from '..';
 import { Namespace, SimpleElement } from '@simple-dom/interface';
+import { castToBrowser, expect } from '@glimmer/util';
 
 export class AttributesTests extends RenderTest {
   static suiteName = 'Attributes';
@@ -239,6 +240,40 @@ export class AttributesTests extends RenderTest {
     this.assertStableNodes();
   }
 
+  @test
+  'handles successive updates to the same value'() {
+    class Model {
+      @tracked value = '';
+    }
+
+    let model = new Model();
+
+    this.render('<input value={{this.model.value}} />', { model });
+    this.assert.equal(this.readDOMAttr('value'), '');
+    this.assertStableRerender();
+
+    let inputElement = castToBrowser(
+      expect(this.element.firstChild, 'expected input to exist'),
+      'input'
+    );
+
+    inputElement.value = 'bar';
+    this.assert.equal(this.readDOMAttr('value'), 'bar');
+
+    model.value = 'foo';
+    this.rerender();
+    this.assert.equal(this.readDOMAttr('value'), 'foo');
+    this.assertStableNodes();
+
+    inputElement.value = 'bar';
+    this.assert.equal(this.readDOMAttr('value'), 'bar');
+
+    model.value = 'foo';
+    this.rerender();
+    this.assert.equal(this.readDOMAttr('value'), 'foo');
+    this.assertStableNodes();
+  }
+
   @test
   'input[checked] prop updates when set to undefined'() {
     this.registerHelper('if', (params) => {

packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts

@@ -1,4 +1,4 @@
-import { Reference, valueForRef, isConstRef } from '@glimmer/reference';
+import { Reference, valueForRef, isConstRef, createComputeRef } from '@glimmer/reference';
 import { Revision, Tag, valueForTag, validateTag, consumeTag } from '@glimmer/validator';
 import {
   check,
@@ -16,7 +16,7 @@ import {
   VMArguments,
   Owner,
   CurriedType,
-  ModifierDefinitionState,
+  ModifierDefinitionState,, Environment
 } from '@glimmer/interfaces';
 import { $t0 } from '@glimmer/vm';
 import { APPEND_OPCODES, UpdatingOpcode } from '../../opcodes';
@@ -237,27 +237,34 @@ APPEND_OPCODES.add(Op.DynamicAttr, (vm, { op1: _name, op2: _trusting, op3: _name
   let attribute = vm.elements().setDynamicAttribute(name, value, trusting, namespace);
 
   if (!isConstRef(reference)) {
-    vm.updateWith(new UpdateDynamicAttributeOpcode(reference, attribute));
+    vm.updateWith(new UpdateDynamicAttributeOpcode(reference, attribute, vm.env));
   }
 });
 
 export class UpdateDynamicAttributeOpcode extends UpdatingOpcode {
   public type = 'patch-element';
 
-  public lastValue: unknown;
+  private updateRef: Reference;
 
-  constructor(private reference: Reference<unknown>, private attribute: DynamicAttribute) {
+  constructor(reference: Reference<unknown>, attribute: DynamicAttribute, env: Environment) {
     super();
-    this.lastValue = valueForRef(reference);
-  }
 
-  evaluate(vm: UpdatingVM) {
-    let { attribute, reference, lastValue } = this;
-    let currentValue = valueForRef(reference);
+    let initialized = false;
 
-    if (currentValue !== lastValue) {
-      attribute.update(currentValue, vm.env);
-      this.lastValue = currentValue;
-    }
+    this.updateRef = createComputeRef(() => {
+      let value = valueForRef(reference);
+
+      if (initialized === true) {
+        attribute.update(value, env);
+      } else {
+        initialized = true;
+      }
+    });
+
+    valueForRef(this.updateRef);
+  }
+
+  evaluate() {
+    valueForRef(this.updateRef);
   }
 }