-
Notifications
You must be signed in to change notification settings - Fork 879
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
Change attribute/property binding syntax to require opt-in to properties. #399
Comments
I'm liking the last option the more I look at it. It places the modifier in a consistent place and away from the other symbols required for a binding ( I think it'll be nice with a lot of bindings, when I typically put each attribute on a separate line: html`<input
id=${id}
.value=${value}
?required=${required}
@change=${onChange}>` There the binding type is very clear. The downside is that |
Definitely the last option. The one right under Proposed Syntax feels very confusing to me, personally. |
How about this version? This is something that we are already using but if possible
|
I personally prefer the clean-slate (everything changes together consistently) including where you put the modifier. Everything else has a bit of hysteresis. |
@motss my problem with that is the run of symbols between the attribute name and value. The modifier gets very lost, imo. |
The bias for me being left to right is how I read, and what @justinfagnani just stated. |
My vote is: html But I haven't really thought it through. |
And regarding a new symbol: I partially agree, but after looking at the examples for a bit I find |
A nice side-effect of One of the painful bits of React is that |
The third option seems the most consistent to me; and as @appsforartists just said, |
Special symbols and characters are something you usually regret over time. It's harder for people to learn. Defaulting to attributes is definitely the right call though! |
I like the Feels like the prefix characters being first would also make things slightly easier when you have to create regexes for tools like html minifiers to ignore certain parts of the templates. It is also nicer having them away from the "make it so!" in best Jean-Luc Picard voice ... |
Overall huge fan of the proposed attribute by default, property with dot prefix. And moving the boolean attribute to prefix for consistency seems good too. I'm on the fence regarding the |
I'm really ambivalent, just throwing out concerns in case any of them might be important. Yeah, if easier to parse, that's a strong argument. On an aesthetic level, I remember the Scala language "guy" explaining why he put types after the variable name, which seems to be a trend. What's important comes first. What type it is comes after, because it's a technical detail. But, yeah, it does mean more non-ascii characters together. |
I'm wondering if It might also save a few unintentional |
Good points there. I really like the |
The @ is also the decorator symbol in JS, or was the decorator feature dropped from spec? Either way, Angular uses @ as a decorator. |
I think it is great if non-symbolic names always produce attributes. It is a very intuitive and easily-taught rule. For that reason, I lean toward either Writing the name with no symbol carries the message that attributes are "raw" like natural HTML. As others have pointed out, it is good to avoid the incorrect impression that the event handlers are natural HTML. Overall, I love the direction. I do sympathize with others who think there are a lot of symbols, but if you are giving up the double meaning of |
Using fancy symbols is kind of the way how Angular does this if I remember correctly. I'm comfortable with @justinfagnani 's idea. html`<input
id=${id}
.value=${value}
?required=${required}
@change=${onChange}>` But then there is another question is that which symbols are much more appropriate to be used so that it makes sense to many of us. We might need a poll for that. I also have an idea like |
how would |
Also, I've seen event names beginning with html`<my-el @@event="${onEvent}"></my-el>` |
I like the prefixes, |
Personally I agree with most of the above (+ for everything, issues with |
I like @Draccoz's idea, but there might be some overlap with generator To me generators suggest RxJS/observable patterns, so repeated events with I'm not sure whether that's a good thing or a potential confusion. |
Almost every symbol means something :D. Unless somebody suggests something really good, we will just have to pick the least confusing out of the confusing ones :(. |
^change or ~change or :change ;-) The latter is like assign to/subscribe to, so works for me |
@KeithHenry, since |
@Lodin Can't agree with that. It may not be JS, but it's still closer to JS than funky templating systems (closer to JSX, which kinda still is JS). That means most of the people using lit (if not all) will be JS devs, and they should not be mislead by different meanings of the symbols they are used to. |
@Lodin if it's not JavaScript, then what it is? (don't say TypeScript 😄) |
How about literal prop({someProp: val})and event({someEvent: fn}) ... this way you can have a map of props or events prop(propsMap) and event(eventsMap) |
Naively speaking. Have yet to use lit-html and the ergonomics of proposed and existing syntax is after all just ergonomics, nothing that we can determine objectively. Maybe just make it user defined syntax with default being the ., ? and @ prefixes? |
@Lodin that wasn't really my point - we can use pretty much anything we want, but ideally it should confuse devs used to JS syntax as little as possible. To me We could make any of them work, but ideally we'd pick one that feels intuitive. Personally I might go for html`<input
id=${id}
.value=${value}
?required=${required}
+change=${onChange}>` |
You could go the opposite direction and make on-event the syntax you keep, so: in-property=${valueForPropertyOnly} Hyphenated attribute names already raise a flag when reading, and polymer users are already familiar with the on- prefix, even if I'm still confused. Progress on a learning curve is a terrible thing to waste. Also, you won't be struggling to find the right single character for: set-property=${writeTraceForProperty} whenever they or other proposals come up. |
@recri That's going to be fun for a lot of existing web components. Most actively avoid
|
@KeithHenry for those used to .NET |
I'd like to restate my argument for why I think it's better to put the property modifiers at the end of the property name rather than the beginning:
|
I'm thinking events should be different. After all, much has been made of "data flows down via props, up via events". So making events look different ties into that. Maybe events should come at the beginning. I'm thinking of the css hover: a:hover. So my new revised proposal is: html`<input id=${id} value:=${value} required?=${required} :change=${onChange}>` Not all the pseudo selectors are event related, but many are (sort of): :focus, :hover, :valid :visited :out-of-range :disabled :enabled :checked Note that pascal and Go use := for assignment. In the case of Go, it allows you to not have to start with var (actually, I don't have that quite right -- implicit types or something. Never programmed in Go, sorry). Update: Evidently, Vue uses the colon prefix to represent attributes (vs properties). Glimmer seems to use @ to represent properties, btw. Anyway, the precedent set by Vue could cause Vue users to find the colon prefix confusing for defining events. Another syntax that has appeal to me: html`<input id=${id} value:=${value} required?=${required} change::${onChange}>` inspired by Rust (sorry, @CaptainCodeman :-) ) on- does appeal to me also, fwiw. If we are trying to attract react/preact users, they would be most comfortable seeing onChange, but the important issue has been raised if you want an event to really start with an uppercase letter, since react/preact uses the upper case to make it more readable, but changes to lower case (I think) under the hood. Could both on and on- be supported, and one would use the latter if needing upper case letters, like on-URLChanged? |
I don't think what any other language (including JS) uses for symbols really matters. This is a DSL to generate HTML templates so that should really be the main focus, not how any symbol is used or what it means elsewhere - so many languages, all different. Having the attribute syntax align with HTML attribute usage is definitely an improvement as part of that. We just want to come up with a clear and simple way to indicate properties and events. |
I like html`<input id=${id} .value=${value} ?required=${required} @change=${onChange}>` Although I don't understand why we need the if (val === true) {
el.setAttribute(attr, "");
} else if (val === false) {
el.removeAttribute(attr);
} else {
el.setAttribute(attr, val);
} If you really want to bind the string representation of the boolean you can still do that explicitly by wrapping it in quotes. I.e.: html`<div active=${true}></div>` would render as <div active></div> While html`<div active="${true}"></div>` would result in <div active="true"></div> This is much closer to what I would expect coming in unbiased (if fact the second case is identical to what would happen in vanilla template literals) and since the whole point was to get rid of special syntax when binding to attributes this is a logical step imo. One could go even further and define special behavior for promises, generators etc as well all of which can be optet out of by simply using quotes. Finally, I think the prefix should only specify the target, while the interpretation of the value could be consistent across targets. I.e. promises could be handled identically for properties, attributes and even event handlers (of course in the latter case the promise would have to resolve to a function). If the expression resolves to something that isn't valid for a given target (e.g. a boolean for an event handler) you'll get a nice, explicit error that tells you exactly what you did wrong. |
@KeithHenry, @Draccoz, @moebiusmania, I suppose, @CaptainCodeman said everything I wanted in the better way. I believe if Anyway, I would vote for the following solution: html`<input id=${id} .value=${value} ?required=${required} @change=${onChange}>` IMO, it is very expressive and consistent solution. BTW, why not to create poll anywhere, e.g. in twitter? |
I do agree with @Lodin and @CaptainCodeman, and my only issue with |
@Draccoz, regarding Slack: if you use triple backtick block for code (or snippet which is even better), it won't create any mentions. And you have to use this block anyway because if you don't Slack will highlight all your string between |
I know that very well, kept reminding people about it all the time (sometimes I get tired of it though and ignore it...). We - slackers - are aware of it, but hell-of-a-lot of newcomers either don't know it or don't care about it, this is the potential source of infinite mentions (EmpressPolymer and couple of others know it very well since Polymer moved to npm with its own namespace ^^). |
@Draccoz Not sure how my proposed solution would be any more magic than what lit already does. Can you elaborate? |
In the current proposal, you can clearly see what is happening, if there is a |
@Draccoz @Lodin Yeah, avoiding However, the |
Thanks for all the fruitful discussion everyone! I haven't said much yet, because so far there's really good reasoning going on and it's very interesting to just see how people think and feel about this and how that even changes rather quickly based on new comments. I definitely missed an opportunity to spread lots of 🚲and 🏠emoji throughout this issue! I wanted to drop a couple of responses, though, maybe most importantly this: from @Lodin:
Mainly, because that might give the impression that the poll winner should "win". There are a lot of inputs to this decision, like how people feel, how intuitive it is, how familiar it is to other syntaxes, how easy it is to parse, consistency, etc. It's not a fully democratic decision. Even if it were, we all know how accurate internet polling is, and who knows what fraction of the future user-base the poll would actually reach. And finally, because this is at least in part a subjective decision, as is all bike-shedding, there's an element of taste and even tastemaking here. We're going to take everything into consideration and try to make a decision that makes a cohesive whole. What's really interesting here is that this is a decision just for the default syntax. The unopinionated core will remain, and it's very easy to make your own syntax. This is the entirety of the Option 3 syntax implementation: export const partCallback =
(instance: TemplateInstance,
templatePart: TemplatePart,
node: Node): Part => {
if (templatePart.type === 'attribute') {
const name = templatePart.name!;
const prefix = name[0];
if (prefix === '.') {
return new PropertyPart(
instance, node as Element, name.slice(1), templatePart.strings!);
}
if (prefix === '@') {
return new EventPart(instance, node as Element, name.slice(1));
}
if (prefix === '?') {
return new BooleanAttributePart(
instance,
node as Element,
name.slice(1),
templatePart.strings!);
}
}
return defaultPartCallback(instance, templatePart, node);
}; Now, we probably don't want a massive proliferation of syntaxes out there, but if some company, major widget library, or framework that uses lit-html wants to have a different opinion here to use across their templates, they can. @bgotink already has an Angular-inspired syntax which has quite a few additional features: https://github.com/bgotink/lit-html-brackets Now some specific points: One way to do somethingI didn't mention this before, but I really want there to be one and only one way to do something. Since we can't do anything but set attributes when there's not a binding, this would require that the default binding is to attributes and that there is no other attribute syntax. So no LHS vs RHS positioningFor properties and events I'm fairly convinced that the LHS is the right position, so that it's clear at a glance that these are not properties. For boolean attributes I could go either way, since it is an attribute and the Events:
|
Thanks for all the input everyone! #398 is merged now. We decided to go with option 2: html`<input id=${id} .value=${value} ?required=${required} @change=${onChange}>` Note that lit-extended.js hasn't changed, so the next release should be non-breaking, but it is deprecated. cc @bgotink who maintains a syntax |
does this also support setting properties if just provided as a string html`<my-el .prop="some-string">` |
@daKmoR No. lit-html doesn't process attributes that don't have bindings. Supporting that might have a perf impact, and would require some API changes. Let me think about this for a little bit... |
This syntax does exactly what you want, without any performance overhead: html`<my-el .prop=${"some-string"}>` I think it's much easier to explain that lit-html interacts with and only with instances of It's a slippery slope. Property setters through attributes are ok, but data bindings are not? It'll just become an arbitrary cutoff point for what 'feels right'. The cutoff for what lit-html interacts with should be |
I totally agree with @ruphin, lit only interacts with template string intersections, the rest is a 100% standard html behavior. If we start messing with those, we end up with a library being far away from the standards (and we aimed at it didn't we?). |
@justinfagnani in |
@klebba that was actually a bug that ignored We've since added the import {ifDefined} from 'lit-html/directives/if-defined.js';
html`<a target="${ifDefined(foo)}">link</a>` |
@justinfagnani thanks for the prompt reply! Would be convenient to be able to accomplish this using template syntax, e.g. |
Agree 100% |
The current syntax of lit-extended sets properties by default and requires opting out of properties to set attributes with the
$
suffix.This has a number of downsides:
viewBox
are harder to implementThis issue is to document the rationale for changing the syntax to set attributes by default and require opt-in to properties with a
.
prefix.Compison
Current syntax:
Proposed syntax:
And for consistency, we could go further and make all binding modifiers be single-symbol prefixes, like:
The text was updated successfully, but these errors were encountered: