Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 task
ajiho opened this issue Jun 26, 2024 · 3 comments · Fixed by #11369
Assignees
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)

Comments

@ajiho
Copy link

ajiho commented Jun 26, 2024

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

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Jun 26, 2024
@bluwy
Copy link
Member

bluwy commented Jun 26, 2024

This is where we handle how attribute values are handled

// Boolean values only need the key
if (value === true && (key.startsWith('data-') || htmlBooleanAttributes.test(key))) {
return markHTMLString(` ${key}`);
} else {
return markHTMLString(` ${key}="${toAttributeString(value, shouldEscape)}"`);
}

="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 b958088#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
Copy link
Author

ajiho commented Jun 26, 2024

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

@matthewp
Copy link
Contributor

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.

@bluwy bluwy added - P3: minor bug An edge case that only affects very specific usage (priority) and removed needs triage Issue needs to be triaged labels Jun 28, 2024
@bluwy bluwy self-assigned this Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants