Skip to content

Conversation

mercuryseries
Copy link
Contributor

@mercuryseries mercuryseries commented Aug 20, 2025

Q A
Bug fix? yes
New feature? no
Docs? no
Issues #
License MIT

Problem

Demo video
before.mov
<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:

<button data-model="title" data-value="" data-action="live#update">Clear</button>
<!-- Results in: null -->

Expected behavior:

<button data-model="title" data-value="" data-action="live#update">Clear</button>  
<!-- Should result in: "" -->
After (demo video)
after.mov

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

@carsonbot carsonbot added Bug Bug Fix Status: Needs Review Needs to be reviewed labels Aug 20, 2025
@carsonbot carsonbot changed the title Fix: Return empty string for data-value="" instead of falling back to null Fix: Return empty string for data-value="" instead of falling back to null Aug 20, 2025
Copy link
Contributor

github-actions bot commented Aug 20, 2025

📊 Packages dist files size difference

ℹ️ No difference in dist packagesFiles.

@smnandre
Copy link
Member

👍

It would indeed be consistent with Stimulus values https://stimulus.hotwired.dev/reference/values#getters

Could you add some tests to prevent any future regression ?

@carsonbot carsonbot changed the title Fix: Return empty string for data-value="" instead of falling back to null [LiveComponent] Fix: Return empty string for data-value="" instead of falling back to null Aug 20, 2025
@smnandre smnandre changed the title [LiveComponent] Fix: Return empty string for data-value="" instead of falling back to null [LiveComponent] Return empty string for data-value="" instead of falling back to null Aug 20, 2025
@smnandre smnandre added the DX label Aug 20, 2025
@Kocal Kocal added Status: Needs Work Additional work is needed and removed Status: Needs Review Needs to be reviewed labels Aug 22, 2025
@mercuryseries
Copy link
Contributor Author

@smnandre I added some tests. I see that some tests failed, but seems not related with the changes. Can you confirm please?

@smnandre
Copy link
Member

It does not feel related to me.. @Kocal wdyt ?

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Work Additional work is needed labels Aug 22, 2025
@smnandre smnandre removed the Bug Bug Fix label Aug 22, 2025
@Kocal
Copy link
Member

Kocal commented Aug 23, 2025

No that's not related, it should be green after a rebase

@Kocal Kocal force-pushed the fix/data-value-empty-string branch from 0a47fc7 to 208ee9a Compare August 23, 2025 06:50
@Kocal Kocal added Bug Bug Fix and removed DX labels Aug 23, 2025
@Kocal Kocal force-pushed the fix/data-value-empty-string branch from 208ee9a to d0c0670 Compare August 23, 2025 07:04
@Kocal
Copy link
Member

Kocal commented Aug 23, 2025

Thank you @mercuryseries.

@Kocal Kocal merged commit b19cee6 into symfony:2.x Aug 23, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug Fix LiveComponent Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants