Skip to content

Commit b19cee6

Browse files
committed
bug #3031 [LiveComponent] Return empty string for data-value="" instead of falling back to null (mercuryseries)
This PR was squashed before being merged into the 2.x branch. Discussion ---------- [LiveComponent] Return empty string for `data-value=""` instead of falling back to null | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Docs? | no <!-- required for new features --> | Issues | # <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead --> | License | MIT ## Problem <details> <summary><b>Demo video</b></summary> https://github.com/user-attachments/assets/31bba6af-8690-4cd9-a5c4-20a561342626 </details> ```html <input type="text" data-model="post.title"> <button type="button" data-model="post.title" data-value="" <!-- HERE --> data-action="live#update" > Clear Title </button> ``` When using `data-value=""` on an element, the current implementation incorrectly returns `null` instead of the intended empty string value. This happens because the condition `if (element.dataset.value)` evaluates to `false` for empty strings (since empty strings are falsy in JavaScript), causing the function to fall through to the final `return null;` statement. ## Impact This behavior prevents developers from explicitly setting empty string values via the `data-value` attribute, which is a common use case for clearing form fields or resetting values to empty states rather than `null`. ### Current behavior: ```html <button data-model="title" data-value="" data-action="live#update">Clear</button> <!-- Results in: null --> ``` ### Expected behavior: ```html <button data-model="title" data-value="" data-action="live#update">Clear</button> <!-- Should result in: "" --> ``` <details> <summary><b>After (demo video)</b></summary> https://github.com/user-attachments/assets/bad5f5f3-1104-4102-a2bc-30264bb3c5bf </details> ## Changes Replace the truthiness check `if (element.dataset.value)` with an explicit attribute existence check `if (element.hasAttribute("data-value"))`. This ensures that any `data-value` attribute, including those with empty string values, are properly processed and returned. ## Manual testing done - ✅ `data-value=""` now returns `""` - ✅ `data-value="test"` still returns `"test"` - ✅ Missing `data-value` attribute still falls back to other value sources - ✅ All existing functionality remains intact Commits ------- d0c0670 [LiveComponent] Return empty string for `data-value=""` instead of falling back to null
2 parents 30c3a2e + d0c0670 commit b19cee6

File tree

3 files changed

+36
-2
lines changed

3 files changed

+36
-2
lines changed

src/LiveComponent/assets/dist/live_controller.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ function getValueFromElement(element, valueStore) {
377377
}
378378
return element.value;
379379
}
380-
if (element.dataset.value) {
380+
if (element.hasAttribute("data-value")) {
381381
return element.dataset.value;
382382
}
383383
if ("value" in element) {

src/LiveComponent/assets/src/dom_utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export function getValueFromElement(element: HTMLElement, valueStore: ValueStore
5151
}
5252

5353
// element is some other element
54-
if (element.dataset.value) {
54+
if (element.hasAttribute('data-value')) {
5555
return element.dataset.value;
5656
}
5757

src/LiveComponent/assets/test/unit/dom_utils.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,40 @@ describe('getValueFromElement', () => {
110110

111111
expect(getValueFromElement(div, createStore())).toEqual('the_value_from_attribute');
112112
});
113+
114+
it('Returns empty string for data-value=""', () => {
115+
const btn = document.createElement('button');
116+
// simulate the attribute explicitly present but empty
117+
btn.dataset.value = '';
118+
// also set a value on the button to ensure data-value takes precedence
119+
btn.value = 'should_not_be_used';
120+
expect(getValueFromElement(btn, createStore())).toBe('');
121+
});
122+
123+
it('Returns "0" for data-value="0" (falsy but valid)', () => {
124+
const el = document.createElement('div');
125+
el.dataset.value = '0';
126+
expect(getValueFromElement(el, createStore())).toBe('0');
127+
});
128+
129+
it('Prefers data-value over value attribute when both are present', () => {
130+
const el = document.createElement('div');
131+
el.dataset.value = 'from_data_value';
132+
el.setAttribute('value', 'from_value_attribute');
133+
expect(getValueFromElement(el, createStore())).toBe('from_data_value');
134+
});
135+
136+
it('Falls back to value attribute when data-value is absent', () => {
137+
const el = document.createElement('div');
138+
el.setAttribute('value', '');
139+
// No data-value set
140+
expect(getValueFromElement(el, createStore())).toBe('');
141+
});
142+
143+
it('Returns null when neither data-value nor value attribute nor value property is present', () => {
144+
const el = document.createElement('div');
145+
expect(getValueFromElement(el, createStore())).toBe(null);
146+
});
113147
});
114148

115149
describe('setValueOnElement', () => {

0 commit comments

Comments
 (0)