Fast: [Proposal] FastElement Property and Attribute Binding Changes

Created on 25 Mar 2020  路  10Comments  路  Source: microsoft/fast

Description

As the features and usage of FastElement have advanced, some early decisions related to the templating syntax and behavior of property and attribute bindings seem less correct. I'm proposing that we make a breaking change to how bindings work in order to create greater consistency, reduce library size, and enable critical scenarios.

Today

Today we have the following syntax:

  • attr=${x => x.value} - Uses the expression to update the value of the property associated with the attr via element.attr = value.
  • $attr=${x => x.value} - Uses the expression to update the value of the attribute via element.setAttribute('attr', value);
  • ?attr=${x => x.value} - Uses the expression to update the value of the attribute using boolean attribute behavior.

Problems

  • Some have found that plain attr= behavior to be unintuitive. They expect this to set the attribute, rather than the property.
  • Some custom element attributes names don't work correctly with attr=. For example, if the attribute is my-attribute, the binding my-expression=${x => x.value} will result in an attempt to set the my-expression property, which doesn't exist. Most likely the property name is myExpression. There's no way for the template compiler to know this. All attr name conversion is based on knowledge of built-ins. So, there's no way to perform a property binding in this case. An explicit attribute binding is needed. This is unintuitive.
  • If an element exposes some settings via properties with no attribute, then there's no way for the binding engine to set those if the property has mixed case property names.

Proposal

  • attr=${x => x.value} - Uses the expression to update the value of the attribute via element.setAttribute('attr', value);
  • :attr=${x => x.value} - Uses the expression to update the value of the property via element.attr = value.
  • ?attr=${x => x.value} - Uses the expression to update the value of the attribute using boolean attribute behavior.
  1. I propose that a binding with no prefix should set the attribute with no name alteration or mapping. It should be set exactly as specified using the setAttribute API.
  2. I propose that the prefix : designate a property binding and that we add support for mixed-case property names.
  3. I propose that we remove the $ prefix, as it will no longer be needed.
  4. I propose that we remove all attribute to property name conversion code, as it will no longer be needed.

Note: The : syntax was chosen for property binding because it is used in other popular libraries like Vue and Aurelia 2. This will be familiar to many people who may adopt the library.

I believe these changes will be more intuitive as well as fix the shortcomings with attributes that don't match property names, and mixed case property names.

Level of Effort

The level of effort to implement this change is relatively slow. It can be done in a single day, most likely. After the change, we'll need to also update our docs and go through all of our current components, updating their templates as appropriate.

We're still early enough in the project that we can make this sort of change. There's a bit of pain for us to update templates, but I believe that it's worth it to avoid the issues mentioned above which could cause us long-term problems or a much more disruptive breaking change later on.

chore

Most helpful comment

FWIW, I just tried setting class attribute with setAttribute and it worked, so it might not be necessary to have a special case even for class:

image

All 10 comments

I think this sounds great - I also found the behavior of attr="foo" setting the property to be a bit confusing at first.

One other detail, I think that if you do a class=${} we should special case that, and set theclassName, since I don't think this is even settable through the attribute API. Any objections to that?

This all looks good to me, I also like the alignment with other libs for some familiarity for developers.

One other detail, I think that if you do a class=${} we should special case that, and set theclassName, since I don't think this is even settable through the attribute API. Any objections to that?

Regarding using class on the <template> - do we run into an issue when setting class names internally and externally? Say a component sets classes on the host, but an author also wants to set classes on the element. Not sure it is 100% related but just a thought I had that I think we want to accommodate if we're not already

FWIW, I just tried setting class attribute with setAttribute and it worked, so it might not be necessary to have a special case even for class:

image

Was just about to follow up with that @nmetulev - I'd prefer to not special case if there isn't a limitation with the attribute.

Thanks for confirming that @nmetulev !

@nicholasrice I think we can go with the setAttribute behavior for class for now. We may need to special case that for host directives like you mention above, because it will clobber external classes without special handling.

I'll move head with these changes. Hopefully I'll have a PR ready by tomorrow.

PR coming soon. I just need to update the docs to match the changes.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

bheston picture bheston  路  3Comments

radium-v picture radium-v  路  5Comments

EisenbergEffect picture EisenbergEffect  路  5Comments

MarcSkovMadsen picture MarcSkovMadsen  路  6Comments

gqio picture gqio  路  3Comments