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

Svelte 5: Spreading attributes to element breaks hidden #11179

Closed
huntabyte opened this issue Apr 15, 2024 · 5 comments · Fixed by #11371
Closed

Svelte 5: Spreading attributes to element breaks hidden #11179

huntabyte opened this issue Apr 15, 2024 · 5 comments · Fixed by #11371
Milestone

Comments

@huntabyte
Copy link
Member

huntabyte commented Apr 15, 2024

Describe the bug

When you have an element setup like this:

<script lang="ts">
	let isHidden = $state(false)
</script>

<div {...restProps} hidden={isHidden}>
</div>

When isHidden changes, hidden becomes "false", which is still hidden.

If you remove the spread obj. from the attributes, if hidden is false, it's removed from the attributes, making the element visible.

<div hidden={isHidden}>
</div>

So the issue is around the inconsistencies, as well as the type Svelte expects for the hidden attribute, which is currently boolean | null | undefined, but should be string | null | undefined.

Reproduction

REPL

Logs

No response

System Info

Svelte 5 REPL environment

Severity

annoyance

@huntabyte huntabyte changed the title Spreading Attributes to element breaks hidden Spreading attributes to element breaks hidden Apr 15, 2024
@huntabyte huntabyte changed the title Spreading attributes to element breaks hidden Svelte 5: Spreading attributes to element breaks hidden Apr 15, 2024
@brunnerh
Copy link
Member

brunnerh commented Apr 15, 2024

hidden is not a boolean attribute, it a) probably should not be treated like a boolean attribute and b) might still need some special attention.

These are valid:

<div hidden > <!-- same as hidden="hidden" -->
<div hidden=""> <!-- same as hidden="hidden" -->
<div hidden="hidden">
<div hidden="until-found">

Toggling between "hidden" and undefined works.
Toggling between "" and undefined does not quite work.

@huntabyte
Copy link
Member Author

huntabyte commented Apr 15, 2024

Understood @brunnerh, thanks for the clarification! I guess what is confusing is that the second div in my repl would lead one to believe that it does work that way... until you try to spread some additional attributes and it suddenly stops working that way!

The attribute is also typed in Svelte as boolean | null | undefined, so unless you're in a REPL it's unlikely you'd ever even try using a string.

@anatolzak
Copy link

@brunnerh currently, the TS type for the hidden property is boolean | null | undefined

@brunnerh
Copy link
Member

until-found is fairly new and not even supported in many browsers.
Sometimes boolean attributes just somehow turn into enumerations 😅.

@Rich-Harris Rich-Harris added this to the 5.0 milestone Apr 21, 2024
@dummdidumm
Copy link
Member

FWIW the same happens in Svelte 4

dummdidumm added a commit that referenced this issue Apr 29, 2024
When spreading attributes, the setters of the element are checked. If they contain the key in question, it's set via that setter. For certain setters on certain elements this didn't work because the element prototype was not HTMLElement, rather a descendant of that (for example HTMLDivElement), which meant that only the setters of the descendant, not the superclass were taken into account. This fixes that by walking up the prototype chain until we find the Element prototype.

fixes #11179
dummdidumm added a commit that referenced this issue Apr 29, 2024
When spreading attributes, the setters of the element are checked. If they contain the key in question, it's set via that setter. For certain setters on certain elements this didn't work because the element prototype was not HTMLElement, rather a descendant of that (for example HTMLDivElement), which meant that only the setters of the descendant, not the superclass were taken into account. This fixes that by walking up the prototype chain until we find the Element prototype.

fixes #11179
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants