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

The problem with enumerated and Booleanish #14301

Open
AlexRMU opened this issue Nov 14, 2024 · 5 comments
Open

The problem with enumerated and Booleanish #14301

AlexRMU opened this issue Nov 14, 2024 · 5 comments

Comments

@AlexRMU
Copy link

AlexRMU commented Nov 14, 2024

Describe the bug

There are several attributes whose values can be enumerated.
However, if the values are string representations of booleans, then the value type becomes Booleanish.

draggable?: Booleanish | undefined | null;

This can lead to similar errors:

Reproduction

https://svelte.dev/playground/98dbee7e667d49028d0d7a662fd58f4e?version=5.1.16

Logs

No response

System Info

-

Severity

annoyance

@dummdidumm
Copy link
Member

This looks like an extension of #12664, which #13327 was supposed to fix - but turns out it didn't really, because we forgot about the static case. I'm not exactly sure what's the best way to deal with this is. We have a couple of options:

  • a) adjust the types: boolean is not allowed anymore, you gotta instead do ="true/false". A breaking change strictly speaking, and doesn't really help when you're not using types. Not a good option IMO
  • b) leave this as is and document it as a caveat, i.e. say that draggable !== draggable={true/false} / {draggable}. One can argue this is correct (since in raw HTML just draggable is also incorrect), but one can also argue that they feel the same.
  • c) make it work by having some special cases for static booleanish attributes that are actually enumerated "true/false"

I'm torn between option b) and c)

@AlexRMU
Copy link
Author

AlexRMU commented Nov 15, 2024

Yes, some errors can only be stumbled upon by accident.
It's about the nuances of the specification and the operation of browsers.
Why are some attributes booleanish and others enumerated?
For example, if you replace draggable with contenteditable in the repl, then everything will work, although both of them are enumerated.
I think it's better to keep the processing logic simple and choose option c).

@dummdidumm
Copy link
Member

Why are some attributes booleanish and others enumerated?

Ask the browser gods, we are as much scratching our heads as you

For example, if you replace draggable with contenteditable in the repl, then everything will work, although both of them are enumerated.

That seems to be a browser quirk where they allow contenteditable="" to mean the same as contenteditable="true", for whatever reason.

@AlexRMU
Copy link
Author

AlexRMU commented Nov 15, 2024

I've studied the specification and I think I understand now:

  • contenteditable (MDN, standard) = "plaintext-only" | "false" | "true" | ""
  • draggable (MDN, standard) = "false" | "true"
view value
attr ""
attr="" ""
attr="true" "true"
attr="string" "string"

contenteditable = contenteditable="" = "true"
draggable = draggable="" = missing/invalid = auto

@Leonidaz
Copy link

This looks like an extension of #12664, which #13327 was supposed to fix - but turns out it didn't really, because we forgot about the static case. I'm not exactly sure what's the best way to deal with this is. We have a couple of options:

  • a) adjust the types: boolean is not allowed anymore, you gotta instead do ="true/false". A breaking change strictly speaking, and doesn't really help when you're not using types. Not a good option IMO
  • b) leave this as is and document it as a caveat, i.e. say that draggable !== draggable={true/false} / {draggable}. One can argue this is correct (since in raw HTML just draggable is also incorrect), but one can also argue that they feel the same.
  • c) make it work by having some special cases for static booleanish attributes that are actually enumerated "true/false"

I'm torn between option b) and c)

I think option c) is probably the best option, if it's not too difficult, since it's highly likely that this is going to keep coming up and would be a "drain" on everyone to deal with submitted issues, including the submitters themselves.

Option b) is likely going to be missed by most in the documentation as it's not something one would be scanning for, and we know how much most of us love carefully reading documentation 😂

Option a) would definitely be way too harsh.

One other option, let's call it d), that would be obvious for devs, but not sure if it's easier than implementing c), is to show a warning if the explicit true / false is missing.

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

No branches or pull requests

3 participants