withastro / astro

The web framework for content-driven websites. ⭐️ Star to support our work!
https://astro.build
Other
43.84k stars 2.28k forks source link

There are different behaviors for custom attributes that do not start with data and attributes that start with data #11342

Open ajiho opened 3 days ago

ajiho commented 3 days ago

Astro Info

View link:https://stackblitz.com/edit/github-x7gejb?file=src%2Fpages%2Findex.astro

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

image What should I do for attributes that do not start with data if I also want to maintain the absence of attribute values? Their performance is different, and in the final generated HTML file, I only want to keep custom instead of custom="true"

What's the expected result?

keep custom instead of custom="true"

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-x7gejb?file=src%2Fpages%2Findex.astro

Participation

bluwy commented 2 days ago

This is where we handle how attribute values are handled

https://github.com/withastro/astro/blob/dc95d674af8792773693d491a1f7520d0260e20b/packages/astro/src/runtime/server/render/util.ts#L117-L122

="true" can only be omitted if the attribute is a known boolean attribute, so custom="true" is expected. However, data-custom (without value) seems odd to me because it's not known to be a boolean attribute.

Support for omitting the value if true for data-* was added in https://github.com/withastro/astro/commit/b958088c3dbabc43f698ff6684e6d54a8ec189f4#diff-3974ff1a07864eae3cb91ceefef360fbc39db051f8036ee2504b9be00764e0ccR226 by @matthewp, which perhaps is a bug? I think we can only omit it if the value is "", not true, which I also confirm is the behaviour from vue and svelte today.

I'm not sure how breaking this is if we change now, but in practice probably not a lot as the way to check the attributes truthiness will still work? (hasAttribute)

ajiho commented 2 days ago

To be honest, I am looking forward to this change so that I can confidently migrate from the Twig template engine to Astro

matthewp commented 1 day ago

I'm surprised we've gone this long without discovering this. @bluwy I concur with your findings, should be omitted only if "" is the value, same for data- or non-data.