Configure attr's with property effects. More robust fix for #3288.

[kevinpschaaf]

No description provided.

[arthurevans]

So, currently if I have a property defined in the properties object (or in a binding, for example), it is automatically queued up for property deserialization.

Does this fix mean that if I have <x-foo some-thing$="{{value}}"> that the property someThing does not get set? And if I have <x-foo some-thing="stringValue"> that someThing does get set?

[kevinpschaaf]

@arthurevans <x-foo some-thing="stringValue"> never becomes a property if someThing is not a property Polymer is managing; that's just a normal attribute.

See #3288 for a description of the bug; for an attribute binding like <x-foo some-thing$="{{value}}">, we were setting the property someThing once at configuration time (but never thereafter), even if someThing wasn't a property Polymer was managing. This is attempting to fix that bug such that we never set someThing if it's not a property Polymer is managing while leaving all other previous behavior as-is (and fixing one edge case related to typed attribute deserialization that we found in the process).

[sorvell]

LGTM.

[kevinpschaaf]

@arthurevans We should probably include a note in the next release notes highlighting the bug in case users had previously relied on it. Basically something like:

Prior to Polymer v1.2.5, a bug existed related to attribute bindings to unknown attributes (specifically, attributes that did not map to properties that Polymer creates accessors for). The bug resulted in the value of the attribute being mirrored to a property of the same name once (but subsequent changes in the attribute were not reflected, which could be very confusing). This was unintentional: Polymer's contract is to only deserialize attributes to properties that polymer is managing, aka properties declared in the properties block or used as dependencies to bindings, observers, or computed functions. Users should review their code to ensure that they were not relying on the unintentional one-time deserialization of unknown attributes. For example, the following will no longer work:

<!-- binding to unknown attribute: will not deserialize to property -->
<button foo$="{{property}}">`

// Passes:
assert(button.getAttribute('foo') == this.property);
// Fails:
assert(button.foo == this.property);

If users need to access the property value of a bound element, they should ensure they are binding to a property as opposed to an attribute:

<!-- binding to unknown property always works -->
<button foo="{{property}}">`

// Passes:
assert(button.foo == this.property);

[David-Mulder]

@kevinpschaaf Is there a way to dynamically set properties based on attributes in that case? An API consumption element of mine accepts all param-* attributes right now which works perfectly (I guess it works well because although I rely on the binding to work, but I do not expect propagation from child to parent or listeners or anything like that).

And the reason it's not a params Object is simply because it's not possible to bind foo.x="{{property}}" so you end up with foo="{{someComputedObjectOfParamsWithLotsOfListeners}}" which is incredibly ugly. And honestly, if you make non-backward compatible changes it would be really great if you guys would start using semantic versioning and update the major version number.

[kevinpschaaf]

I understand your point of view of this feeling non-backward compatible but it truly was an unintended side-effect of a bug, and its unfortunate you were relying on the buggy behavior. However, we can't force every user to go through a manual step of opting into a new major change for every bug fix because someone might have been relying on a side-effect of the bug.

To be clear on the nature of the bug, although bound attributes were deserialized to otherwise unknown properties once, subsequent changes to the attribute would never update the property -- this is why the initial reporter filed the bug in the first place, and highlighted an undocumented behavior we were not aware of until then. As the docs on attribute deserialization state, properties should be declared in the properties object in order to ensure deserialization.

If your element was reading this['param-foo'], this['param-bar'], this['param-ziz'] at startup by relying on deserialization of the undeclared properties, it can still just read this.getAttribute('param-foo'), this.getAttribute('param-bar'), this.getAttribute('param-ziz') to achieve the same effect.

[David-Mulder]

@kevinpschaaf: So right now it will still do the binding, but the binding will only bind to the attribute? Because the parsing of paramProperties is happening in the execute of an <my-api> element.

And as far as semver, yes the idea is indeed that if a bugfix is likely going to break stuff (as you seemed to be aware of) that you postpone it till the next major release or make the release it is in a major release. I have not so far encountered a library which has introduced so many breaking changes in minor releases (.#1 numbering of keys in the core, styles disappearing after changes in paper-styles, etc.).

[kevinpschaaf]

In Polymer you can either bind to a property or an attribute:

<div foo="[[bar]]"> will set the value of bar to the foo property on the div.

<div foo$="[[bar]]"> will set the value of bar to the foo attribute on the div.

That's all the binding does, either set a property or an attribute.

It is the behavior of the target element to decide what happens when either an attribute or property is set on it. As you can see with a <div>, it knows nothing about the foo attribute, and so if you read the div.foo property it will be undefined -- the div doesn't have some contract to reflect every attribute value to the same-named property, only attributes it defines on its API. However, if you instead bound to the title attribute, this is something that the API contract of <div> states it will deserialize to the title property, and if you read div.title you'll see the property. This is the exact behavior that Polymer intends to enforce for your element. The target element of a binding decides which attributes to deserialize to properties (via its properties configuration) -- it has nothing to do with binding at all. It was a bug in how we efficiently flow binding data from host to child at startup (an implementation detail of the binding system) that caused the incorrect deserialization.

If you have no reason to have the param data available on an attribute (as opposed to a property), then you should simply bind to the param-* property via a property binding as opposed to an attribute binding, which will avoid the need to call getAttribute to get the value since the binding will set the properties directly. In practice you should virtually never need to use attribute binding except for certain attributes on native elements for compatibility/legacy reasons, since binding to properties is always faster and more direct. e.g.:

<my-el param-foo="[[foo]]" param-bar="[[bar]]">

Hope that helps.

[David-Mulder]

@kevinpschaaf I feel somewhat bad for hijacking this thread for a specific issue, but regardless, what do you mean with:

then you should simply bind to the param-* property

because as far as I know it's not possible to have wildcarded properties. I mean, I know everything else you said, but the thing this incomplete feature/'bug' made possible was exactly that: allowing wildcarded property names (an API consumption element doesn't know what parameters each call accepts, because that would be redundant). It looks however like there is a different way to bind to wilcarded property names based on your comment, but I have no idea how that's supposed to work.

[kevinpschaaf]

Sorry I didn't mean use a wildcard literally, but that you can bind param-whatever using property bindings, as I gave in the example:

With property binding the host will set the properties listed directly on the child:

<my-el param-foo="[[foo]]" param-bar="[[bar]]">

As opposed to attribute binding (which would require cooperation from <my-el> to get the attribute value to its property):

<my-el param-foo$="[[foo]]" param-bar$="[[bar]]">

You can bind to paramFoo and paramBar properties of <my-el> even if <my-el> hasn't declared those properties. This is because a binding simply directs the host to set the property defined in the binding to the child with no knowledge or care of what the child will do with the property.

[arthurevans]

Can you clarify whether you're actually seeing your element break with 1.3.0? If so, can you supply a repro for your problem?

To be clear, any API element that relied on this side effect would only ever work once, since as the bug describes, the bogus property was only set during element initialization and never updated.

[jonnenauha]

@kevinpschaaf does this change the custom element boolean "toggle" logic. I'm thinking of upgrading from 1.2.4 and have this type of code

<paper-tabs scrollable$="[[ myBool ]]">

I'm not certain from the docs if that $ should be there if both side are a Boolean, esp. after this change. I think there was some problem in earlier versions, so that if scrollable had any value it made itself true, as the bool attribute was there. It was not checking if the value of it was "false". Adding the $ will remove the attribute if the right side goes to false and it works ok.

I think other than that I'm safe as have always used $= if I just want to bind into a custom attribute value. I believe this wont break with this change and the attribute gets updated every time the value changes? In code I'm retrieving the custom attr value with elem.getAttribute("my-data"), never via elem.myData (which I believe this change focused on).

<div my-data$="[[ obj.id ]]"> // the attr still updates if obj.id changes, right?

[kevinpschaaf]

Right, in the paper-tabs case, where scrollable is a declared property of paper-tabs, you should use normal property binding (scrollable="[[myBool]]"), as properties are a more efficient way of passing data than attributes.

In the case of binding to a random custom attribute like my-data, you should use attribute binding (my-data$="[[obj.id]]") -- the change here was avoiding the data-binding system making a property on the target for an attribute binding on its own; this should be the purview of the target element, i.e. it's up to the target element implementation to react to an attribute change by deserializing it to a property if it so chooses.

Also note that Polymer will happily bind any value to a property on an element regardless of whether the element cares about the property or not. So e.g. <div my-data="[[ obj.id ]]"> will result in div.myData property being set.