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] Literal 0 (zero) cannot be set as prop without giving it any default #10854

Closed
harrisi opened this issue Mar 21, 2024 · 11 comments · Fixed by #11539
Closed

[Svelte 5] Literal 0 (zero) cannot be set as prop without giving it any default #10854

harrisi opened this issue Mar 21, 2024 · 11 comments · Fixed by #11539
Milestone

Comments

@harrisi
Copy link

harrisi commented Mar 21, 2024

Describe the bug

If a component exposes a literal 0 (zero) as a prop, it cannot be set by the parent without setting a default value (even of undefined).

One alternative is to just use a string representation (i.e., '0'), but it's odd that this doesn't work for 0 but does for other numbers.

The compiled code is odd - it's like it's treating 0 as special, even though it's actually receiving the string representation - '0'.

Reproduction

repl

Logs

No response

System Info

repl

Severity

annoyance

@dummdidumm dummdidumm added this to the 5.0 milestone Mar 21, 2024
@trueadm
Copy link
Contributor

trueadm commented Mar 21, 2024

Should we even permit numeric literals as props?

@harrisi
Copy link
Author

harrisi commented Mar 21, 2024

Maybe not. There was some dislike expressed in the $props.bindable discussion about restricting syntax or not, which is interesting here. Restricting just numbers seems reasonable, since you can't set them from a parent anyway. I don't know if the discussion about restricting strings is relevant at the same time. Here are some weird uses. This is fine to do in Svelte 4 (and Vue, it seems), too, but binding to arbitrary strings that aren't valid JavaScript identifiers is new. I don't see why it shouldn't be allowed, even though it's pretty weird, unlike numbers, which are really just the string representation (except 0).

@dummdidumm
Copy link
Member

In Svelte 4 it is possible to use bascially anything as an identifier, you could then access it through $$props['0-ysc%%gibberish']. I hope noone actually took advantage of that, and I would be ok with us trimming down the range of allowed characters, even if these are (probably?) all valid html attribute characters. I would also be ok with us not caring as long as we don't see any impact beyond making it work. The 0 thing is probably just a bug somewhere we do something like !variable instead of variable != undefined.
cc @Conduitry who probably has more thoughts on this.

@trueadm
Copy link
Contributor

trueadm commented Mar 21, 2024

It's less about if it's technically feasible, but rather is it a good idea to begin with. Having a 0 prop is not only completely unreadable, but it's also a nightmare to use and reference at the other side nicely. We should just disallow it.

<Component 0={data} 1={something_else} /> is just not something I feel Svelte should promote.

@dummdidumm
Copy link
Member

dummdidumm commented Mar 21, 2024

We don't promote it and I hope noone does this - given that we want Svelte 5 to be upgradeable as much as possible, I vote for keep allowing it for 5.0 but add a compiler warning (we already have one for : in prop names), and make a note that we change this for 6.0

@harrisi
Copy link
Author

harrisi commented Mar 21, 2024

I should also note that since attributes are just strings, they can be non-ASCII. I'm not sure where the line should be drawn, but allowing °é…• but not '5' seems arbitrary.

@7nik
Copy link

7nik commented Mar 22, 2024

I think the prop name should match js var names except for allowing -: [$_a-zA-Z][-$_a-zA-Z0-9]*.

@harrisi
Copy link
Author

harrisi commented Mar 22, 2024

I feel like limiting it gets weird. While $foo is a valid identifier in JavaScript, it's not in Svelte 5 runes mode. So that probably shouldn't be allowed. On top of that, my-var isn't a valid identifier but it is allowed. Disallowing it seems like a weird choice since kebab-case is standard for html elements.

I think any restriction just for the sake of not allowing weird props is a bad idea. They are just strings, which are valid in object destructuring. Disallowing literal numbers in the destructuring pattern seems reasonable since there's no way to use them, but other than that, why restrict it?

@7nik
Copy link

7nik commented Mar 22, 2024

Indeed, it makes sense to limit the first symbol of a name to [a-zA-Z].

Kebab-case is required to bypass and/or intercept HTML attributes. I think adequate people won't define custom props in kebab-case, except for maybe custom elements.

Using weird names is a way to shoot in your leg, and the code will only confuse people. In any case, it's possible to pass a prop with whatever name via spreading and resting props.

@harrisi
Copy link
Author

harrisi commented Mar 22, 2024

It seems there's already a limitation on attribute names for custom components: export const regex_illegal_attribute_character = /(^[0-9-.])|[\^$@%&#?!|()[\]{}^*+~;]/;. However, since it's a block list instead of an allow list, ė…‰ and whatnot are allowed. But they don't seem to work properly in svelte for some reason.

I think matching the spec as closely as possible would be best.

EDIT: It seems as though this is the definition for valid attribute names:

NameStartChar ::= "_" | [a-z] | [#xC0-#xD6] | [#xD8-#xF6] | [#xF8-#x2FF] | [#x370-#x37D] | [#x37F-#x1FFF] | [#x200C-#x200D] | [#x2070-#x218F] | [#x2C00-#x2FEF] | [#x3001-#xD7FF] | [#xF900-#xFDCF] | [#xFDF0-#xFFFD] | [#x10000-#xEFFFF]
NameChar ::= NameStartChar | "-" | "." | [0-9] | #xB7 | [#x0300-#x036F] | [#x203F-#x2040]
Name ::= NameStartChar (NameChar)*

Related, valid custom element names look like this:

PotentialCustomElementName ::= [a-z] (PCENChar)* '-' (PCENChar)*
PCENChar ::= "-" | "." | [0-9] | "_" | [a-z] | #xB7 | [#xC0-#xD6] | [#xD8-#xF6] | [#xF8-#x37D] | [#x37F-#x1FFF] | [#x200C-#x200D] | [#x203F-#x2040] | [#x2070-#x218F] | [#x2C00-#x2FEF] | [#x3001-#xD7FF] | [#xF900-#xFDCF] | [#xFDF0-#xFFFD] | [#x10000-#xEFFFF]

and must not be one of these:

  • annotation-xml
  • color-profile
  • font-face
  • font-face-src
  • font-face-uri
  • font-face-format
  • font-face-name
  • missing-glyph

sources:

https://html.spec.whatwg.org/multipage/custom-elements.html

https://www.w3.org/TR/xml/#NT-Name

@harrisi
Copy link
Author

harrisi commented Mar 22, 2024

I suppose it doesn't really make sense to follow the spec for web components since attributes can already be non-conformant (capital letters).

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.

4 participants