-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
Property syntax using curly braces and quotes is inconsistent. #7925
Comments
This dates back a long time ago (I think Svelte 1?) where people would generally use HTML syntax highlighting when editing Svelte components, so we encouraged people to use |
A counter argument to changing this is that you can do this today: |
How is that a counter argument? This is just string interpolation in exactly the way I expect it to work. |
I'm sorry, forget my comment, didn't read thoroughly enough. Note to self: this would also need adjustment in prettier-plugin-svelte |
Given how big of a breaking change this is I suggest to first adjust |
Either add some types or configure your prettier/linter to format or yell at you. |
If you’re passing the string value to a component property which expects a number then you’d get an error message immediately. I guess jsdoc can solve this problem by specifying property types if you’re not using typescript. |
Revisiting this, I still don't think we should change this behavior. It's arguably an edge case and AFAIK some people still use the prettier plugin's HTML strict mode due to editor limitations which means quotes are always applied. I'm in favor of kicking this can down the road further. |
Sure, it's just a minor thing anyway. |
Here's what I think we should do: in dev mode, print a warning if a component prop or element
(Challenge: indicating where in the source code the offending prop was.) If we do that, we can fix this weirdo behaviour in Svelte 6. |
Does Svelte still support this? The downside of disallowing x="{y}" syntax is that it breaks functionality in other frameworks. For example, HEEX templated in Phoenix/Elixir already use the exact syntax with curly braces, so, when I do something like this: This is <mycomponent customval={43}/> Phoenix renders it as
<mycomponent customval="43"/> But, my component still works. With this change proposed, now I will have to end up doing something like: <div data-customval="43">
<mycomponent/>
</div> And then have to fetch it inside my component. |
I don't understand what a different template language has to do with this change? Could you elaborate? What is |
@trueadm when implementing this we need make sure that we have some way to know from the AST that the attribute was quoted. This is important for editor tooling to know. (I guess you need to adjust the AST in some way anyway, because the compiler also needs to know) |
How do you mean? Can you explain more please on why we need to change the AST? |
Because in Svelte 6 |
I meant, there are other frameworks which depend on the syntax supporting the "" quoted syntax. Assume I have a component like below. This is my <menu items={items}/> Previously, this also used to work: <menu items="{items}"/> Now, my understanding is that it will no longer work post this particular issue being closed? As this quoted syntax support above allowed other frameworks outside of Svelte to support passing items from backend code which had a very similar syntax to Svelte. The one I quoted originally is from Phoenix/Elixir (reference). |
I'm afraid I'm not following. |
Re AST changes: rather than adding a export interface Attribute extends BaseNode {
type: 'Attribute';
name: string;
value:
| { type: 'boolean' }
| { type: 'expression'; expression: Expression }
| { type: 'text'; chunks: Array<Text | ExpressionTag> };
metadata: {};
} Then, if an attribute has a |
I'm looking at this more closely right now and it's a bit tough. From a code completion (language tools) and formatting (prettier) angle:
For 1) we can look into completion not adding quotes automatically, which would at least help for IDEs that use the Svelte language server. Still, these measures likely don't catch all cases / reach all people. Furthermore there's a much bigger problem I see: once Svelte 6 is out with the warning removed and the formatting rule adjusted to just leave quotes as-is, it will become much more likely that people accidentally add quotes and then are wondering why their code does not work. The problem, somewhat unique to Svelte, is that the way to pass the value as-is (
I also haven't seen anyone complain about the behavior from the angle of "I wanted to make this a string, but it's not, because the quotes are essentially ignored". So to summarize, this change removes a slight inconsistency but introduces a much more relevant and more likely source of error. This makes me hesitant to make the change this way. Alternatives:
|
In Svelte 5, attributes are never quoted, because this will mean "stringify this attribute value" in a future Svelte version Related to sveltejs/svelte#7925
In Svelte 5, attributes are never quoted, because this will mean "stringify this attribute value" in a future Svelte version Related to sveltejs/svelte#7925
In a future version, that will mean things are getting stringified, which is a departure from how things work today, therefore a warning first. Related to #7925
…12479) * parse `foo={bar}` attribute as `value: { type: 'ExpressionTag', .. }` (i.e. don't wrap in an array) * warn on quoted single-expression-attributes * breaking: warn on quotes single-expression attributes in runes mode In a future version, that will mean things are getting stringified, which is a departure from how things work today, therefore a warning first. Related to #7925 * Update .changeset/plenty-items-build.md * Apply suggestions from code review * missed a spot --------- Co-authored-by: Rich Harris <rich.harris@vercel.com>
The warning is in place for components and custom elements now, moving this to 6.0 so we think about the next steps for that major when it comes to that. |
Thanks @Rich-Harris and @dummdidumm for taking a closer look! |
@dummdidumm I think the warning should be extended to elements, e.g. with the string conversion logic, event handlers in quotes do not make much sense: <button onclick="{increment}">
clicks: {count}
</button> Or with certain properties like <input type="file" bind:files="{files}" /> |
It's not added there because for |
There's a solid chance I'm not following this well yet for how to mitigate / update for the change - but one case that is now throwing errors is when using pug via svelte-preprocess. With this you need to quote some attributes to be able to use them (and can optionally control how they are encoded with the = or =! operator); e.g. - <template lang="pug">
CustomComponent(propName='{ callFunction }')
CustomComponent(propName!='{ () => doSomething = true }')
</template> Both of these syntax throw a warning cases like this (on a Component, but not on a standard element like a button). https://github.com/sveltejs/svelte-preprocess/blob/main/docs/preprocessing.md#pug Is there are way to disable the warning project-wide (or file wide, rather than multiple inline |
Recently a compilerOptions: {
warningFilter: ({ code }) => code != 'attribute_quoted',
}, Though the preprocessor probably needs to be adjusted if it currently always outputs the quotes 🤔 |
Describe the bug
There is this special syntax
prop="{value}"
that does not cause string interpolation but instead just passes the value of the expression as is. This is inconsistent with how properties and string interpolation behave in all other cases.As I see it, as soon as quotes are used, the value should be an interpolated string, no magic. Obviously, this would be a significant breaking change, so this is more of a suggestion for the future.
Reproduction
REPL
Logs
No response
System Info
Severity
annoyance
The text was updated successfully, but these errors were encountered: