breaking: use quotes to differentiate custom element properties from attributes

[Rich-Harris]

Closes #12624 — I figured it would be worth having a branch to play with so that we can give it a proper spin.

Only two tests needed to be updated. The attribute-custom-element-inheritance test was very easy to fix because the warnings told me exactly what I needed to do. The other one was a tiny bit trickier because the attribute is reflected as a prop (squelching any warning) but needs to be present as an attribute in order to influence the CSS. Definitely a bit of an edge case, but flagging it nonetheless (and the "{red || undefined}" is admittedly weird).

On the whole, I like it — it gives control that doesn't exist today, and guides people towards the correct outcomes except in rather esoteric cases. I definitely like it more than ideas like attr:foo and prop:bar for the reasons given here: #12624 (comment)

Dynamic custom elements are currently a TODO (though the current heuristic obviously works fine for the existing tests)

Svelte 5 rewrite

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: 6226b85

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]

If we do these changes, then in the interest of backwards compat I propose to only do it in runes mode. That way existing code doesn't require a massive cleanup / you're not blocked if the code isn't under your control.
I'm also wondering if we should keep the "no property given, set as attribute instead" behavior - does it hurt / have unwanted consequences in any way?

If we go with this we also need to update prettier-plugin-svelte to leave quotes on custom elements as-is (right now it's always removed)

[Rich-Harris]

Am on board with making this a runes-mode-only thing

I'm also wondering if we should keep the "no property given, set as attribute instead" behavior

Not sure I understand?

we also need to update prettier-plugin-svelte

Mind if I leave that to you? I'm unfamiliar with the codebase, so it'll be quicker that way...

[dummdidumm]

I'm also wondering if we should keep the "no property given, set as attribute instead" behavior

Not sure I understand?

I mean that <c-e foo={bar} /> doesn't mean cE.foo = bar but still uses set_custom_element_data(cE, 'foo', bar), and the function checks "oh there is no property foo, so I'll set it as an attribute instead". That way you can mostly use that form without thinking about it too much, and you only need to use foo="{bar}" in case you explicitly want it as an attribute, even if it has a corresponding property.

Mind if I leave that to you?

Yeah no problem, was more a note to myself that we (I) need to adjust that if we go with this change.

[Rich-Harris]

I mean that <c-e foo={bar} /> doesn't mean cE.foo = bar

If there's no foo accessor then you'll get a dev time warning suggesting that you do foo="{bar}" instead. I don't think we should go back to being loosey-goosey, because then you have no way to say 'actually I would like this to be assigned as a prop, and I don't care whether there's an accessor or not'.

For example you might have a prop that doesn't have a corresponding accessor because it doesn't affect rendering (e.g. it's only referenced inside an event handler). It's a rare case but it could happen. With the PR as it currently is you can just leave a note for yourself explaining that:

<!-- svelte-ignore custom_element_invalid_property — foo is a non-reactive property, not an attribute -->
<c-e foo={bar} />

[geoffrich]

There's also a case where the property isn't defined yet if the custom element definition is being lazy-loaded -- so I agree with Rich that being explicit makes sense.

Old post of mine about that case: https://css-tricks.com/using-custom-elements-in-svelte/#aa-lazy-loading-custom-elements

[Rich-Harris]

To be fair things would still be broken in that case — I wonder if we need to add some customElements.whenDefined logic to handle lazy-loaded custom elements. Annoying bloat in most cases of course, but that's the price you pay for using a broken primitive. Would be a separate chunk of work to this PR

[Rich-Harris]

Closing as we decided to delay this until 6.0