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

Distinguish between properties and attributes on custom elements #12624

Open
Rich-Harris opened this issue Jul 26, 2024 · 27 comments
Open

Distinguish between properties and attributes on custom elements #12624

Rich-Harris opened this issue Jul 26, 2024 · 27 comments

Comments

@Rich-Harris
Copy link
Member

Describe the problem

One of the many reasons custom elements are a pain in the neck is that it's often rather ambiguous what's supposed to be an attribute and what's supposed to be a property. Some custom elements 'reflect' attributes back to properties and vice versa, but there's no universally agreed upon convention. If you're doing low-level DOM manipulation you can decide whether you want to do element.setAttribute(name, value) or element[name] = value, but everyone stopped writing code like that a decade ago.

It's a messy, flawed design, and different frameworks deal with it in different ways. Preact and Svelte both treat something as a property if prop in node, and an attribute otherwise. Vue does the same thing but additionally gives you a way to force something to be a prop (I don't think you can force something to be an attribute) with some interesting syntax choices:


image

Lit leans even harder into syntax, with ? and . and @ prefixes for boolean attributes, properties and event handlers respectively:


image

I think the explicit control is a good thing, but I think we can provide it more naturally.

Describe the proposed solution

#7925 and #12479 point the way. We're moving to a world in which quoted props on components are stringified:

<script>
  let object = { a: 1, b: 2, c: 3 };
</script>

<!-- in Svelte 6, this will mean `data="[object Object]"` -->
<Thing data="{object}" />

(The reason the quotes are ignored today is historical: in the distant past before Svelte editor extensions, when components were typically written in .html files, it avoided crazypants syntax highlighting.)

What if we did something similar for custom elements?

<foo-bar someprop={somevalue} someattribute="{someothervalue}"></foo-bar>

In other words if something is quoted, it's an attribute, otherwise it's a prop. We might need to change some Prettier defaults, but other than that this should be a relatively straightforward change. (I don't see any need to distinguish between 'boolean attributes' and properties like Lit does — they're all just properties.)

It is, of course, a breaking change. We would need to decide whether to do this in 5.0, or have a bunch of warnings and change it in 6.0. Ordinarily the conservative thing to do would be to delay the breaking change, but since it's not currently possible to differentiate properties and attributes I think there's a reasonable case for doing it sooner. As such I've given it a 5.0 milestone for now.

Importance

nice to have

@trueadm
Copy link
Contributor

trueadm commented Jul 26, 2024

I actually love this a lot. Simplifies a ton of things along the way too.

@MarcusOtter
Copy link

And I assume to pass a string as a property the syntax would be this? (if I don't have it in a variable)

<foo-bar someprop={"my string here"}></foo-bar>

@ottomated
Copy link
Contributor

My only qualm with this is that it seems hard to teach and unintuitive. Why does wrapping something in quotes mean that it's an attribute?

@henrikvilhelmberglund
Copy link

My only qualm with this is that it seems hard to teach and unintuitive. Why does wrapping something in quotes mean that it's an attribute?

Because HTML attributes use quotes style="color:red;". Imo makes perfect sense, if it has quotes outside of {} eg. "{}" it's an attribute (like normally in HTML), if it does not it's a prop.

@ottomated
Copy link
Contributor

ottomated commented Jul 26, 2024

if it has quotes outside of {} eg. "{}" it's an attribute (like normally in HTML), if it does not it's a prop.

But in the case <foo-bar prop="foo" />, prop would be a property, right? What about in the case <foo-bar prop="{foo} a b" />?

To me, "{}" just signifies string interpolation, which is why I think that getting rid of the confusing legacy behavior is great – I don't like replacing it with a confusing new behavior.

It's kind of like if `${foo}` had a special behavior in javascript.

@Rich-Harris
Copy link
Member Author

<foo-bar prop="foo" />prop is an attribute, because "foo". If you want a property, prop={"foo"}

<foo-bar prop="{foo} a b" /> — also an attribute

@ottomated
Copy link
Contributor

What would the recommended way to do string interpolation for a property? <foo prop={`${foo} a b`} />?

@Rich-Harris
Copy link
Member Author

Exactly, just like in normal JavaScript:

props = {
  prop: `${foo} a b`
};

@MrHBS
Copy link

MrHBS commented Jul 27, 2024

Yes I love this ❤️

@dummdidumm
Copy link
Member

What about spreading properties? Does this always count as setting them as properties?

What about regular elements? I've seen (very few, rare) cases where people wanted things explicitly as an attribute or property.

As for timing: Since this is an uncommon case and can easily solved through e.g. an action, I'm in favor of taking our time with it and don't rush this into 5.0

@rChaoz
Copy link
Contributor

rChaoz commented Aug 6, 2024

How about boolean attributes? REPL

<input type="text" disabled={disabled}>
<!-- or -->
<input type="text" {disabled}>

Also, I believe it will be very easy to confuse the following, especially for new users:

<my-element a="{variable}" b={"text"} />

If we are going to make a breaking change, why not just force props to use a prefix, like :, or the same for attributes:

<my-element a={variable} :b="text" />

...and the same rules we knew so far about {} apply just the same.

Side now: I totally agree with arg="{variable}" converting the argument to a string, I wouldn't mind seeing that sooner than latter, but it would still be pretty breaking.

@Theo-Steiner
Copy link
Contributor

Theo-Steiner commented Aug 6, 2024

I've been meaning to file an issue for this for ages, but never got around (totally not me bumping into this last year and promising to file an issue for it... 😅 )

Vue does the same thing but additionally gives you a way to force something to be a prop (I don't think you can force something to be an attribute) with some interesting syntax choices:

there's actually a syntax in vue for explicitly attributes as well: <my-foo bar.attr="passed as attribute"></my-foo> (although I don't think it is really documented anywhere?)

I think if it is set in quotes, it is an attribute, otherwise it's a property is a very elegant solution, but a few issues came to mind

  • it's quite hard to spot
  • it breaks with expecations of svelte 4 users
  • having an equivalent to the prop shorthand <my-foo {bar}/> would be problematic
    • for bar="hidden", <my-foo "{bar}" /> will be translated to which one?
      a. <my-foo hidden="hidden" />
      b. <my-foo hidden />
  • one shortcoming that came to mind:
    • since {"foo": "bar"} is equivalent to {foo: "bar"}, spreading can only ever set props
    • in vue, one could theoretically spread {"foo.attr": "bar"} vs {"foo": "bar"} to explicitly set a prop
      • (I don't think this is implemented in vue though)

Since this is an edge case, trading a less elegant/simple API for explicitness / programmatic control could be worth the tradeoff?
Maybe not reinventing the wheel, but picking either lit's .foo & .foo? or vue's foo.prop & foo.attr actually could be a good solution here, since people are already familiar with it and it wouldn't conflict with the existing way of doing things

@Rich-Harris
Copy link
Member Author

What about spreading properties? Does this always count as setting them as properties?

There's basically three options:

  1. {...stuff} spreads props
  2. {...stuff} spreads attributes (or rather, does what set_attributes does on regular elements)
  3. It uses the current heuristic

Given that a custom element has a finite number of props and a potentially infinite number of attributes, I would argue that (1) doesn't make sense — if you want to carve the props out of stuff, you can easily do that. That also results in the desired behaviour when spreading things like class.

I think (2) is the best option — it's consistent with how normal elements are handled, and easier to explain. We could augment it with a 'did you mean to do <foo-bar baz={stuff.baz} />?'-type warning, which we could also use to help people make sure they're using the right thing when not spreading. This would alleviate the concerns around discoverability.

How about boolean attributes?

For consistency I think we'd want to treat disabled="{disabled}" as an attribute in the context of a custom element (but this would raise the sort of runtime warning I just described, if the element in question implemented a disabled prop, or it was a property on HTMLElement.prototype like hidden).

This does create an inconsistency with <input disabled="{disabled}">, because we always treat that as a property (because there's no ambiguity when dealing with known properties of built-in elements). But I think we could separately consider compiler warnings in those cases, nudging people towards disabled={disabled} (or {disabled}) instead.

Maybe not reinventing the wheel, but picking either lit's .foo & .foo? or vue's foo.prop & foo.attr actually could be a good solution here

Thanks, I hate it 🤪

foo.prop and foo.attr are just terrible syntax IMHO — dot notation already means something in JS, and this looks like we're setting the attr property of the foo object or something. Really feels like a hasty compromise that was necessitated by constraints we don't share (like being able to read templates from HTML). .foo isn't quite as egregious, but it's still an ugly anomaly when it's only required for custom elements. Aesthetics matter!

I do think that if we started to nudge people towards a similar distinction for normal elements (by warning on quoted boolean properties like <input disabled="{disabled}">) it would soon feel like second nature, though I think the proposal still has value without that.

@Rich-Harris
Copy link
Member Author

Rich-Harris commented Aug 22, 2024

Just checked what we currently do...

<a-b x={1} y="2"></a-b>
<c-d {...stuff}></c-d>

...and we use set_attributes for spread props on custom elements, i.e. option (2). Past us clearly agreed at least with that part of the reasoning. So I guess this proposal basically boils down to this:

export default function App($$anchor) {
  var fragment = root();
  var a_b = $.first_child(fragment);

- $.set_custom_element_data(a_b, "x", 1);
- $.set_custom_element_data(a_b, "y", "2");
+ $.set_custom_element_property(a_b, "x", 1);
+ $.set_custom_element_attribute(a_b, "y", "2");

  var c_d = $.sibling(a_b, 2);
  let attributes;

  $.template_effect(() => attributes = $.set_attributes(c_d, attributes, { ...stuff }, undefined, true));
  $.append($$anchor, fragment);
}

...along with an implementation of set_custom_element_property and set_custom_element_attribute that issues an ignorable warning if it looks like you picked the wrong form.

@dummdidumm
Copy link
Member

Yes and no - set_attributes does check if there's a property on the element and uses that, and only falls back to set_attribute if there isn't. set_custom_element_data does the same.

This distinction between custom elements and regular elements and how string attributes are set will require a lot of explanation, and I'm not sure the nuance is easily grasped. If anything, this makes me lean towards aligning it and meaning "string attributes will always be set through set_attributes, regardless of whether it's a custom element or a regular element".
But this would be a pretty huge breaking change, which we cannot do on a whim. And people might have learned from past experiences that there's no difference, and memory muscle will make them use quotes when they shouldn't, and because there's a difference now prettier will no longer format on into the other.
Even though it looks a bit ugly I therefore also lean towards a prefix, making use of our existing : syntax: attr:... and prop:... (which you could also use within spreading, like { 'attr:...: ..., 'prop:...': ... }`). The use cases for this are rare enough that a bit more to type is worth the improved readability.

@Rich-Harris
Copy link
Member Author

which you could also use within spreading, like { 'attr:...: ..., 'prop:...': ... }`

That strikes me as very odd, to be honest. It's a big departure from how directives normally work...

<!-- one of these things is not like the others! -->
<CustomElementWrapper on:eventname attr:foo prop:bar />

...and it gets particularly weird if you want to reference the value inside the wrapper for whatever reason:

let props = $props();

// i can easily imagine people being confused as to why `props.foo` and `props.bar` are undefined
let foo = props['attr:foo'];
let bar = props['prop:foo'];

@Rich-Harris
Copy link
Member Author

Opened #12981 so we can try this out

@dummdidumm
Copy link
Member

You're right, spreading is probably best handled through a option 2 then, but my point about readability stands

@PatrickG
Copy link
Member

Moving from <img src={someSrc} /> to <special-img src={someSrc} /> might be confusing.

Rich-Harris added a commit that referenced this issue Aug 26, 2024
@Rich-Harris
Copy link
Member Author

There are three possibilities:

  1. <special-img> has a src accessor and no src attribute — it works as expected, no changes required
  2. <special-img> has both a src accessor and an attribute ('reflected', in the jargon) — it works as expected, no changes required
  3. <special-img> has a src attribute that is not reflected, in which case Svelte will print the warning telling you that it needs to be an attribute rather than a property

So in the worst case — 3 — you're guided towards the correct outcome and you're learning something about how the element is implemented.

To me the more interesting case is 2. Maybe you need src to be an attribute, because you have some CSS like this...

special-img:not([src]) {
  outline: 5px solid red;
}

...or because you need it for document.querySelector(...) or whatever. Today, it's impossible (at least without an action, which is cumbersome) — Svelte would assume src was supposed to be a prop, because a src prop exists, and would omit the attribute.

With this proposal, it's trivial. As far as I'm concerned it's all upside.

@PatrickG
Copy link
Member

Sounds good.
And how would the SSR output look like? I guess it would be handled like a normal html element, so there would be a src attribute, right?

@Rich-Harris
Copy link
Member Author

That's a great point, I didn't really think about SSR in the PR but it's another point in this proposal's favour. At the moment we have to assume everything is an attribute, even if that results in prop="[object Object]". That's no longer true (except for spread attributes, where we still have to make that assumption). Will update the PR

@dummdidumm
Copy link
Member

What do we do about dash-case? It's somewhat common to use dash-case on attributes and convert them into camelCase on the corresponding attribute.

<c-e a-property={true}></c-e>

You likely don't want this as cE['a-property'] = true. On the other hand, with $props() it's somewhat straightward to define props like that (let { 'a-property': aProperty } = $props(). Should that mean that we have a ['a-property'] property? Do we also create a aProperty automatically?

@Rich-Harris Rich-Harris modified the milestones: 5.0, 6.0 Aug 30, 2024
@Rich-Harris
Copy link
Member Author

We decided to punt on this until 6.0

@dummdidumm
Copy link
Member

@trueadm said that React had a similar syntax to distinguish between properties and attributes way back, and it was very confusing to people when to use which. This may happen here aswell, especially given that the quotes have slightly different meanings on other kinds of elements and components.

@CosmoMyzrailGorynych
Copy link

CosmoMyzrailGorynych commented Jan 13, 2025

Alloooo 👋
Let me draw your attention to sveltejs/svelte-preprocess#660 and especially to my comments there.
Shortly, I think it is against Svelte's vision, and it will also break my project that uses Pug in practice. And I'm (probably) not the only Pug user.
A better alternative is going in Svelte style: creating a prop: directive similar in usage to bind:.

@Conduitry
Copy link
Member

Whether the quotes are too magic is a matter that we'll have to sort out at some point. But, to be honest, interactions with Pug aren't really going to enter into the decision.

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

Successfully merging a pull request may close this issue.