From 9c06bf3be89b0191d3134adb35fbe5d2fcfc193f Mon Sep 17 00:00:00 2001
From: Edoardo Cavazza <edoardo.cavazza@chialab.it>
Date: Mon, 29 Apr 2024 16:59:52 +0200
Subject: [PATCH] Prevent calling `fromAttribute` when attribute set by
 property.

---
 .changeset/giant-otters-battle.md |  5 ++++
 src/Component.ts                  | 41 ++++++++++++++++++++++---------
 src/property.ts                   | 24 ------------------
 test/component.spec.js            | 10 +++++++-
 4 files changed, 44 insertions(+), 36 deletions(-)
 create mode 100644 .changeset/giant-otters-battle.md

diff --git a/.changeset/giant-otters-battle.md b/.changeset/giant-otters-battle.md
new file mode 100644
index 00000000..51b3f54e
--- /dev/null
+++ b/.changeset/giant-otters-battle.md
@@ -0,0 +1,5 @@
+---
+"@chialab/dna": patch
+---
+
+Prevent calling `fromAttribute` when attribute set by property.
diff --git a/src/Component.ts b/src/Component.ts
index 637002dc..8fcf70fc 100644
--- a/src/Component.ts
+++ b/src/Component.ts
@@ -20,7 +20,7 @@ import {
     defineProperties,
     getProperties,
     getProperty,
-    reflectAttributeToProperty,
+    getPropertyForAttribute,
     reflectPropertyToAttribute,
     removeObserver,
     type PropertyConfig,
@@ -115,6 +115,11 @@ export const extend = <T extends HTMLElement, C extends { new (...args: any[]):
          */
         _updateScheduled = false;
 
+        /**
+         * The property that changed.
+         */
+        _changedProperty: keyof this | null = null;
+
         /**
          * A flag to indicate component instances.
          * @returns True if the element is a component.
@@ -235,6 +240,11 @@ export const extend = <T extends HTMLElement, C extends { new (...args: any[]):
          */
         disconnectedCallback() {}
 
+        /**
+         * Invoked each time the component has been updated.
+         */
+        updatedCallback() {}
+
         /**
          * Invoked each time one of the Component's attributes is added, removed, or changed.
          *
@@ -243,7 +253,19 @@ export const extend = <T extends HTMLElement, C extends { new (...args: any[]):
          * @param newValue The new value for the attribute (null if removed).
          */
         attributeChangedCallback(attributeName: string, oldValue: null | string, newValue: string | null) {
-            reflectAttributeToProperty(this, attributeName, newValue);
+            const property = getPropertyForAttribute(this, attributeName);
+            if (!property) {
+                return;
+            }
+
+            const { name, attribute, fromAttribute } = property;
+            if (name === this._changedProperty) {
+                return;
+            }
+
+            if (attribute && fromAttribute) {
+                this[name] = fromAttribute.call(this, newValue);
+            }
         }
 
         /**
@@ -254,14 +276,11 @@ export const extend = <T extends HTMLElement, C extends { new (...args: any[]):
          * @param newValue The new value for the property (undefined if removed).
          */
         stateChangedCallback<P extends keyof this>(propertyName: P, oldValue: this[P] | undefined, newValue: this[P]) {
+            this._changedProperty = propertyName;
             reflectPropertyToAttribute(this, propertyName, newValue);
+            this._changedProperty = null;
         }
 
-        /**
-         * Invoked each time the component has been updated.
-         */
-        updatedCallback() {}
-
         /**
          * Invoked each time one of a Component's property is setted, removed, or changed.
          *
@@ -274,7 +293,9 @@ export const extend = <T extends HTMLElement, C extends { new (...args: any[]):
             oldValue: this[P] | undefined,
             newValue: this[P]
         ) {
+            this._changedProperty = propertyName;
             reflectPropertyToAttribute(this, propertyName, newValue);
+            this._changedProperty = null;
         }
 
         /**
@@ -284,8 +305,7 @@ export const extend = <T extends HTMLElement, C extends { new (...args: any[]):
          * @returns The inner value of the property.
          */
         getInnerPropertyValue<P extends keyof this>(propertyName: P): this[P] {
-            const property = getProperty(this, propertyName, true);
-            return this[property.symbol as keyof this] as this[P];
+            return this[getProperty(this, propertyName, true).symbol as keyof this] as this[P];
         }
 
         /**
@@ -295,8 +315,7 @@ export const extend = <T extends HTMLElement, C extends { new (...args: any[]):
          * @param value The inner value to set.
          */
         setInnerPropertyValue<P extends keyof this>(propertyName: P, value: this[P]) {
-            const property = getProperty(this, propertyName, true);
-            this[property.symbol as keyof this] = value;
+            this[getProperty(this, propertyName, true).symbol as keyof this] = value;
         }
 
         /**
diff --git a/src/property.ts b/src/property.ts
index 4233d1b9..0497672a 100644
--- a/src/property.ts
+++ b/src/property.ts
@@ -572,30 +572,6 @@ export const reflectPropertyToAttribute = <T extends ComponentInstance, P extend
     }
 };
 
-/**
- * Reflect attribute value to property.
- *
- * @param element The node to update.
- * @param attributeName The name of the changed attribute.
- * @param newValue The new value of the attribute (null if removed).
- */
-export const reflectAttributeToProperty = <T extends ComponentInstance>(
-    element: T,
-    attributeName: string,
-    newValue: string | null
-) => {
-    const property = getPropertyForAttribute(element, attributeName);
-    if (!property) {
-        return;
-    }
-
-    // update the Component Property value
-    const { name, attribute, fromAttribute } = property;
-    if (attribute && fromAttribute) {
-        element[name] = fromAttribute.call(element, newValue);
-    }
-};
-
 /**
  * Populate property declaration using its field descriptor.
  * @param declaration The declaration to update.
diff --git a/test/component.spec.js b/test/component.spec.js
index 2c5fe5dd..bb775b7b 100644
--- a/test/component.spec.js
+++ b/test/component.spec.js
@@ -706,9 +706,11 @@ describe(
                 });
 
                 describe('attribute and properties sync', () => {
-                    let element;
+                    let element, fromAttributeSpy;
 
                     beforeAll(() => {
+                        fromAttributeSpy = vi.fn();
+
                         let TestElement = class TestElement extends DNA.Component {
                             constructor(...args) {
                                 super(...args);
@@ -739,6 +741,7 @@ describe(
                             [
                                 DNA.property({
                                     fromAttribute(value) {
+                                        fromAttributeSpy();
                                         return parseInt(value) * 2;
                                     },
                                     toAttribute(value) {
@@ -842,6 +845,11 @@ describe(
                             element.convertion = 4;
                             expect(element.getAttribute('convertion')).toBe('2');
                         });
+
+                        it.only('should not convert attribute if set by property', () => {
+                            element.convertion = 4;
+                            expect(fromAttributeSpy).not.toHaveBeenCalled();
+                        });
                     });
                 });
             });