fix: throw on invalid attribute expressions

[paoloricciuti]

Svelte 5 rewrite

Closes #11734

Initially i went with: if there's not a quote mark and the value returned is more than a single expression there's a problem. But there's some weird and probably old svelte that you can write that was failing

<input foo=text{variable} />

that will result in

<input foo="textvalueofvariable" />

so i had to check for if we are in a mustache with parser.match('{').

Tests are all passing and it does seem to work so i guess my assumption that mustache tags can only return one expression is correct. If it is a string then the closing tag either is a quote or is the monstrosity up above and we can't really do much about this error imho.

Please note that the Svelte codebase is currently being rewritten for Svelte 5. Changes should target Svelte 5, which lives on the default branch (main).

If your PR concerns Svelte 4 (including updates to svelte.dev.docs), please ensure the base branch is svelte-4 and not main.

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests and linting

• Run the tests with pnpm test and lint the project with pnpm lint

[changeset-bot]

🦋 Changeset detected

Latest commit: f320272

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
svelte	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[dummdidumm]

TBH I'm not totally sure if we should even error here. As you point, out foo=asd{x} is equivalent to foo="asd{x}", and therefore foo=asd{x}} is equal to foo="asd{x}}" which is valid. It's for sure an edge case so I could live with it because 99% of all cases it's a mistake. @Rich-Harris thoughts?

[paoloricciuti]

TBH I'm not totally sure if we should even error here. As you point, out foo=asd{x} is equivalent to foo="asd{x}", and therefore foo=asd{x}} is equal to foo="asd{x}}" which is valid. It's for sure an edge case so I could live with it because 99% of all cases it's a mistake. @Rich-Harris thoughts?

This will not error on foo=asd{x}} tho. It will only error on foo={asd}somethingelse but yeah the same could be applied here too (i just think the intent for foo={asd}somethingelse is clearer and imho it's fine to error out)

[Rich-Harris]

I think we should error on foo=asd{x}} and foo=asd{x}, honestly. I think expecting people to use quotes when the attribute value includes { or } is reasonable — foo=asd{x} is just as likely to be a mistaken version of foo=asd {x} as anything.

It's also just inconsistent that foo=asd{x} is considered okay but foo={x}asd isn't.

Is that a breaking change too far?

[dummdidumm]

You're right that it's inconsistent, and I would be ok with requiring quotes in this situation. I fear that breaking change though - can we parse it like before but move the error to the validation phase for runes mode only? i.e. only in runes mode it's an error

[paoloricciuti]

You're right that it's inconsistent, and I would be ok with requiring quotes in this situation. I fear that breaking change though - can we parse it like before but move the error to the validation phase for runes mode only? i.e. only in runes mode it's an error

I think it's doable...let me try.

P.s. this means that

<button
  onclick={() => console.log('hello')}}
  another={'hello'}}
>
  click
</button>

will have the same error as today in rune mode right?

[paoloricciuti]

A couple of things:

1. Is it possible to store some metadata in the AST or is not recommended? Because the info if it's a Mustache or a string it's only at parse time...i can recover it in other ways but that would basically mean do a parsing during validation and it feels wrong.
2. I can also do attribute.start+attribute.name.length !== attribute.value[0].start but it feels even wronger 😄

[Rich-Harris]

You can know if something is quoted like so:

function is_quoted(attribute) {
  if (attribute.value === true) return false;
  return attribute.value.at(-1).end !== attribute.end;
}

[paoloricciuti]

You can know if something is quoted like so:

function is_quoted(attribute) {
  if (attribute.value === true) return false;
  return attribute.value.at(-1).end !== attribute.end;
}

Ok so it's fine to do it like this (which is simpler but similar to what i come up with 😄 thanks)

[dummdidumm]

Thank you!