Merge pull request #1272 from glimmerjs/backport-fix-attribute-updating

[BUGFIX] Fix reactivity of dynamic attributes
glimmerjs · Feb 16, 2021 · 5f66198 · 5f66198
2 parents 7cd5291 + d470003
commit 5f66198
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 16 deletions.
diff --git a/packages/@glimmer/integration-tests/test/attributes-test.ts b/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) => {

diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts
@@ -529,7 +529,7 @@ function setDeferredAttr(
       .elements()
       .setDynamicAttribute(name, valueForRef(value), trusting, namespace);
     if (!isConstRef(value)) {
-      vm.updateWith(new UpdateDynamicAttributeOpcode(value, attribute));
+      vm.updateWith(new UpdateDynamicAttributeOpcode(value, attribute, vm.env));
     }
   }
 }

diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts b/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,
@@ -8,7 +8,7 @@ import {
   CheckNode,
   CheckMaybe,
 } from '@glimmer/debug';
-import { Op, Option, InternalModifierManager } from '@glimmer/interfaces';
+import { Op, Option, Environment, InternalModifierManager } from '@glimmer/interfaces';
 import { $t0 } from '@glimmer/vm';
 import { ModifierDefinition } from '../../modifier/interfaces';
 import { APPEND_OPCODES, UpdatingOpcode } from '../../opcodes';
@@ -159,27 +159,34 @@ APPEND_OPCODES.add(Op.DynamicAttr, (vm, { op1: _name, op2: trusting, op3: _names
   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);
   }
 }