use both element property and attribute to set the value on the element

Elson Correia · Elson Correia · commit ae52a911e4bd · 2024-12-08T00:31:26.000-05:00
diff --git a/docs/documentation/capabilities/form-controls.md b/docs/documentation/capabilities/form-controls.md
@@ -21,6 +21,7 @@ class TextField extends WebComponent {
         'value',
         'placeholder',
         'pattern',
+        'disabled',
         'required',
         'error',
     ]
@@ -30,26 +31,26 @@ class TextField extends WebComponent {
     value = ''
     pattern = ''
     required = false
+    disabled = false
     error = 'Invalid field value.'
 
     handleChange = (value) => {
+        this.value = value
         // dispatch a change event with the input field value
         this.dispatch('change', { value })
     }
 
     // render the component content
     render() {
+        const { error, ...inputAttrs } = this.props
+
         return html`
             <input
+                ${inputAttrs}
                 part="text-input"
                 type="text"
                 ref="input"
                 onchange="${(event) => this.handleChange(event.target.value)}"
-                placeholder="${this.props.placeholder}"
-                disabled="${this.props.disabled}"
-                pattern="${this.props.pattern}"
-                required="${this.props.required}"
-                value="${this.props.value}"
             />
         `
     }
@@ -148,7 +149,7 @@ class TextField extends WebComponent {
 
     formAssociatedCallback(form) {
         // register value received from props
-        this.internals.setFormValue(this.props.value());
+        this.internals.setFormValue(this.props.value(), false);
     }
 }
 ```
@@ -173,6 +174,7 @@ class TextField extends WebComponent {
 
     handleChange = (value, report = true) => {
         this.internals.setFormValue(value);
+        this.value = value;
 
         const [inputField] = this.refs['input'];
 
@@ -206,7 +208,7 @@ class TextField extends WebComponent {
     ...
 
     formAssociatedCallback(form) {
-        this.internals.setFormValue(this.props.value());
+        this.handleChange(this.props.value(), false);
     }
 
     ...
@@ -227,7 +229,7 @@ class TextField extends WebComponent {
     ...
 
     formDisabledCallback(disabled) {
-        this.refs['input'][0].disabled = disabled;
+        this.disabled = disabled;
     }
 
     ...
@@ -245,8 +247,7 @@ class TextField extends WebComponent {
     ...
 
     formResetCallback(form) {
-        this.handleChange('')
-        this.refs['input'][0].value = ''
+        this.handleChange('', false)
     }
 
     ...
@@ -270,8 +271,7 @@ class TextField extends WebComponent {
         if (mode == 'restore') {
             // expects a state parameter in the form 'controlMode/value'
             const [controlMode, value] = state.split('/');
-            this.handleChange(value)
-            this.refs['input'][0].value = value
+            this.handleChange(value, false)
         }
     }
 
@@ -290,6 +290,7 @@ class TextField extends WebComponent {
     static observedAttributes = [
         'value',
         'placeholder',
+        'disabled',
         'pattern',
         'error',
         'required',
@@ -316,6 +317,7 @@ class TextField extends WebComponent {
     placeholder = ''
     value = ''
     pattern = ''
+    disabled = false
     required = false
     error = 'Invalid field value'
 
@@ -324,24 +326,23 @@ class TextField extends WebComponent {
     }
 
     formDisabledCallback(disabled) {
-        this.refs['input'][0].disabled = disabled
+        this.disabled = disabled
     }
 
     formResetCallback() {
-        this.handleChange('')
-        this.refs['input'][0].value = ''
+        this.handleChange('', false)
     }
 
     formStateRestoreCallback(state, mode) {
         if (mode == 'restore') {
             const [controlMode, value] = state.split('/')
-            this.handleChange(value)
-            this.refs['input'][0].value = value
+            this.handleChange(value, false)
         }
     }
 
     handleChange = (value, report = true) => {
         this.internals.setFormValue(value)
+        this.value = value
 
         const [inputField] = this.refs['input']
 
@@ -357,17 +358,15 @@ class TextField extends WebComponent {
     }
 
     render() {
+        const { error, ...inputAttrs } = this.props
+
         return html`
             <input
+                ${inputAttrs}
                 part="text-input"
                 type="text"
                 ref="input"
                 onchange="${(event) => this.handleChange(event.target.value)}"
-                placeholder="${this.props.placeholder}"
-                disabled="${this.props.disabled}"
-                pattern="${this.props.pattern}"
-                required="${this.props.required}"
-                value="${this.props.value}"
             />
         `
     }
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
     "name": "@beforesemicolon/markup",
-    "version": "1.2.0",
+    "version": "1.3.0",
     "description": "Reactive HTML Templating System",
     "engines": {
         "node": ">=18.16.0"
diff --git a/src/html.spec.ts b/src/html.spec.ts
@@ -828,6 +828,35 @@ describe('html', () => {
 			expect(countUpSpy).toHaveBeenCalledTimes(1);
 			expect(countUpSpy).toHaveBeenCalledWith(expect.any(Event));
 		})
+		
+		it('should handle input value', () => {
+			const [value, setValue] = state("");
+			
+			const temp = html`
+			  <input type="text" value="${value}" ref="input" />
+			`.render(document.body);
+			
+			expect(document.body.innerHTML).toBe('<input type="text" value="">');
+			
+			const [inputField] = temp.refs['input'] as HTMLInputElement[];
+			
+			expect(inputField.getAttribute('value')).toBe('')
+			expect(inputField.value).toBe('')
+			
+			setValue('works')
+			
+			jest.advanceTimersToNextTimer(5)
+			
+			expect(inputField.getAttribute('value')).toBe('works')
+			expect(inputField.value).toBe('works')
+			
+			setValue('')
+			
+			jest.advanceTimersToNextTimer(5)
+			
+			expect(inputField.getAttribute('value')).toBe('')
+			expect(inputField.value).toBe('')
+		})
 	})
 	
 	it('should handle primitive attribute value', () => {
@@ -854,7 +883,7 @@ describe('html', () => {
 		el.render(document.body)
 		
 		expect(document.body.innerHTML).toBe(
-			'<div data-test-map="[object Map]" data-test-val="[object Object]"></div>'
+			'<div data-test-map="{}" data-test-val="{&quot;val&quot;:12}"></div>'
 		)
 	})
 	
diff --git a/src/utils/set-element-attribute.ts b/src/utils/set-element-attribute.ts
@@ -13,26 +13,33 @@ export const setElementAttribute = (
     value: unknown
 ) => {
     if (value !== undefined && value !== null) {
-        // take care of web component elements with prop setters
-        if (el.nodeName.includes('-') && !isPrimitive(value)) {
-            const descriptor =
-                // describe the property directly on the object
-                Object.getOwnPropertyDescriptor(el, key) ??
-                // describe properties defined as setter/getter by checking the prototype
-                Object.getOwnPropertyDescriptors(Object.getPrototypeOf(el))[key]
+        const descriptor =
+            Object.getOwnPropertyDescriptor(el, key) ??
+            // describe properties defined as setter/getter by checking the prototype
+            Object.getOwnPropertyDescriptors(Object.getPrototypeOf(el))[key]
+        const isWritable =
+            descriptor?.writable || typeof descriptor?.set === 'function'
+        // Using setAttribute() to modify certain attributes, most notably value in XUL, works inconsistently,
+        // as the attribute specifies the default value. To access or modify the current values, you should use
+        // the properties. For example, use elt.value instead of elt.setAttribute('value', val).
+        // https://stackoverflow.com/questions/29929797/setattribute-doesnt-work-the-way-i-expect-it-to
+        if (isWritable) {
+            // @ts-expect-error Cannot assign to X because it is a read-only property.
+            el[key] = value
+        }
 
-            if (descriptor?.writable || typeof descriptor?.set === 'function') {
-                // @ts-expect-error Cannot assign to X because it is a read-only property.
-                if (el[key] !== value) el[key] = value
-            } else {
-                const v = jsonStringify(value)
-                if (v !== el.getAttribute(key)) el.setAttribute(key, v)
-            }
+        const strValue = jsonStringify(value)
 
-            return
+        if (
+            // in case !isWritable or setting the property did not also update the attribute
+            strValue !== el.getAttribute(key) &&
+            // only set primitive values
+            // only set non-primitive values on non-components
+            // only set non-primitive values on web components if the property is !isWritable
+            (isPrimitive(value) || !el.nodeName.includes('-') || !isWritable)
+        ) {
+            el.setAttribute(key, strValue)
         }
-
-        if (value !== el.getAttribute(key)) el.setAttribute(key, String(value))
     } else {
         el.removeAttribute(key)
     }

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "@beforesemicolon/markup",`
`3`		`- "version": "1.2.0",`
	`3`	`+ "version": "1.3.0",`
`4`	`4`	`"description": "Reactive HTML Templating System",`
`5`	`5`	`"engines": {`
`6`	`6`	`"node": ">=18.16.0"`