Skip to content

Commit

Permalink
Prevent calling fromAttribute when attribute set by property.
Browse files Browse the repository at this point in the history
  • Loading branch information
edoardocavazza committed Apr 29, 2024
1 parent 8ae7c63 commit 9c06bf3
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 36 deletions.
5 changes: 5 additions & 0 deletions .changeset/giant-otters-battle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@chialab/dna": patch
---

Prevent calling `fromAttribute` when attribute set by property.
41 changes: 30 additions & 11 deletions src/Component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
defineProperties,
getProperties,
getProperty,
reflectAttributeToProperty,
getPropertyForAttribute,
reflectPropertyToAttribute,
removeObserver,
type PropertyConfig,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
*
Expand All @@ -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);
}

Check warning on line 268 in src/Component.ts

View check run for this annotation

Codecov / codecov/patch

src/Component.ts#L256-L268

Added lines #L256 - L268 were not covered by tests
}

/**
Expand All @@ -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;

Check warning on line 279 in src/Component.ts

View check run for this annotation

Codecov / codecov/patch

src/Component.ts#L279

Added line #L279 was not covered by tests
reflectPropertyToAttribute(this, propertyName, newValue);
this._changedProperty = null;

Check warning on line 281 in src/Component.ts

View check run for this annotation

Codecov / codecov/patch

src/Component.ts#L281

Added line #L281 was not covered by tests
}

/**
* Invoked each time the component has been updated.
*/
updatedCallback() {}

/**
* Invoked each time one of a Component's property is setted, removed, or changed.
*
Expand All @@ -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;

Check warning on line 296 in src/Component.ts

View check run for this annotation

Codecov / codecov/patch

src/Component.ts#L296

Added line #L296 was not covered by tests
reflectPropertyToAttribute(this, propertyName, newValue);
this._changedProperty = null;

Check warning on line 298 in src/Component.ts

View check run for this annotation

Codecov / codecov/patch

src/Component.ts#L298

Added line #L298 was not covered by tests
}

/**
Expand All @@ -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];

Check warning on line 308 in src/Component.ts

View check run for this annotation

Codecov / codecov/patch

src/Component.ts#L308

Added line #L308 was not covered by tests
}

/**
Expand All @@ -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;

Check warning on line 318 in src/Component.ts

View check run for this annotation

Codecov / codecov/patch

src/Component.ts#L318

Added line #L318 was not covered by tests
}

/**
Expand Down
24 changes: 0 additions & 24 deletions src/property.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 9 additions & 1 deletion test/component.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -739,6 +741,7 @@ describe(
[
DNA.property({
fromAttribute(value) {
fromAttributeSpy();
return parseInt(value) * 2;
},
toAttribute(value) {
Expand Down Expand Up @@ -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();
});
});
});
});
Expand Down

0 comments on commit 9c06bf3

Please sign in to comment.