Allow explicit control of setting attributes vs properties

[justinfagnani]

Compiling template syntaxes that allow users to specify whether an attribute or property is to be set, like Polymer or Angular 2, requires wrapping all primitive values in objects to force property setting. It'd be a lot friendlier of an API to just accept attributes and properties separately in elementOpen.

[sparhami]

The problem is that you would need to have an API that is something like:

elementOpen(tagName, key, statics, attrsLength,
  attrName, attrValue,
  attrNameTwo, attrValueTwo,
  ...
  propertyName, propertyValue,
  ...)

We could definitely add a function that applies just attributes or just properties. It might look something like:

elementOpen(tagName, key, statics);
applyAttributes(attrName, attrValue, ....);
applyProperties(propertyName, propertyValue, ...);

[justinfagnani]

Well... I think taking attribute name/value pairs as alternating varargs is a bit of an odd API. Why not just take an array of key values pairs (I suggest this in #115).

[sparhami]

That would cause memory allocation for each element call on each render pass, which we are purposefully trying to avoid. If that is really an API that you need, you could just create a small wrapper that does an .apply to the elementOpen, applyAttributes and applyProperties functions.

[justinfagnani]

Hmm... I'd be very careful about assuming the allocation behavior of VMs these days, especially in cases like this where you're accessing arguments in the receiver.

It's possible that a similar allocation has to be made for large argument lists and/or when you access them via arguments, and/or that allocation sinking (which V8's had for a while now, not sure about other VMs) would elide the list because it never escapes the stack. I can ask at least some of the V8 team how it handles cases like this.

Did you measure that the argument list API result in less GC pauses or faster render?

[jridgewell]

Before getting too far into discussing the name/value pair API, we recently landed an attribute mutator API that'll allow you to define how to set certain attribute/props. Would this suit your purposes?

iDOM.mutators.attributes.attrName = function(el, name, value) {
  el.setAttribute(name, value);
};

iDOM.mutators.attributes.propName = function(el, name, value) {
  el[name] = value;
};

In fact, we even expose the default applyAttr and applyProp, so you all you'd need is:

iDOM.mutators.attributes.attrName = iDOM.defaults.attributes.applyAttr;
iDOM.mutators.attributes.propName = iDOM.defaults.attributes.applyProp;

[justinfagnani]

I can't immediately see how to use that API, who makes the decision whether to call attrName or propName? How would I translate elementOpen('x-foo', null, null, ['id', 'a'], ['foo', bar]) (where the first list is attributes and the second is properties)?

[jridgewell]

These are defined beforehand. IE, if you know you always want to set id as a property, you're able to define a function mutators.attribute.id that does it:

mutators.attribute.id = function(element, name, value) {
  // Here, `name` = 'id'
  element.id = value;
};

The same goes for foo, or any other attribute. Moreover, there's a special __all mutator that'll be called for any attribute that doesn't have a specific attribute:

mutators.attributes.__all = function(element, name, value) {
  if (shouldSetAsProp(element, name, value)) {
    element[name] = value;
  } else {
    element.setAttribute(name, value);
  }
};

---

Tangent: If you wanted to get really crazy, you define a "separator" that separates the properties from the attribute in the arguments:

elementOpen('div', 'key', statics, 'attr', 'value1', separator, separator, 'prop', 'value2');

Since iDOM will call __all with (<div>, separator, separator), you could determine attributes and props based on that.

[justinfagnani]

In these templates we only know per source attribute whether you want to assign to an attribute or property. In Polymer attributes are suffixed with a $, properties use a plain name. Angular 2 uses parens and brackets. Polymer example:

<input value$="a"></input><x-foo value="{{bar}}"></x-foo>

input/value is an attribute, but x-foo/value is a property.

Interpreting this requires code like this:

    if (node.nodeType === Node.ELEMENT_NODE) {
      let attributes = getAttributes(node); // gets all attributes ending in '$'
      let properties = getProperties(node); // gets all attributes not ending in '$'
      idom.elementOpen(node.tagName, null, null, ???);

Maybe I can do something with __all:

mutators.attributes.__all = function(element, name, value) {
  if (name.endsWith('$') {
    element.setAttribute(name.substring(0, name.length-1), value);
  } else {
    element[name] = value;
  }
};

I'll try this, however it seems like a complicated and hard to grok API to me.

[jridgewell]

Maybe I can do something with __all:

That was one of the reasons for __all. 😉

I'll try this, however it seems like a complicated and hard to grok API to me.

Is it the mutator interface? Or the name/value pairs? I'd love suggestions to make it easier to understand.

[sparhami]

Another option might be something like:

elementOpen('div', 'key', statics, 'attributes', [...], 'properties', [...]);

mutators.attributes.attributes = function(element, name, value) {
  // value is an array
}

mutators.attributes.properties = function(element, name, value) {
  // value is an array
}

Though you would want to check for a diff yourself before trying to apply it.

[jridgewell]

Another option might be something like:

Ah, I didn't even think of that!

[sparhami]

On second thought, not a good idea since we would hold on to the arrays when we do our caching when checking for diffs.

[justinfagnani]

Is it the mutator interface? Or the name/value pairs? I'd love suggestions to make it easier to understand.

1. It's not local. I know at the point I'm calling elementOpen() whether I want to write a property or attribute. Going through an indirection setup in a configuration object is not obvious. The elementOpen, elementClose, text API is very easy to understand. Additional functions in this vein would be easy as well.
2. I'm not sure in what template syntaxes the attribute / property distinction is known globally. The ones I'm familiar with don't, and I don't see how it would work. I don't think you can always just set known HTML attributes as attributes because in some built-in cases you need to set properties. Neither can you assume unknown or custom element attributes should be properties, since sometimes you need to write attributes for styling and selection. In general the idea of globally pre-configuring attribute vs property just doesn't seem useful to me.
3. It looks like a global configuration. What happens when two template libraries in the same context are both using incremental-dom and have conflicting configurations? In the upcoming web components world, and seeing as incremental-dom is a compile target, I have to assume this will be common.

If it really is that much of an impact to allocate Arrays, then I think a function-based API like attr() would work, say... prop().

If micro-optimizations are important, it's even possible that in some limited cases that inlining and loop-unrolling could remove some of the cost of the extra function calls (maybe the same types of cases that hopefully kick in when you're iterating over arguments). Speaking of, are there benchmarks we can try out?

[justinfagnani]

Another need on the properties side is to be able to declare static properties. In the templates I deal with, attribute vs property and static vs dynamic are completely orthogonal.

[jridgewell]

The elementOpen, elementClose, text API is very easy to understand. Additional functions in this vein would be easy as well.

I'm not sure I like this direction, as it would require changes to consider attributes and properties fundamentally different. With the current configuration step, anything from Polymers attr$ declaration to Angular's awkward [bind] syntax can be supported.

But, it does have merits. This would allow formatters with attributes and properties, since we can extend the attr API (the elementOpen API is too limited currently).

I'm not sure in what template syntaxes the attribute / property distinction is known globally

My hope was this would be used to properly support setting/unsetting id as a property, syncing form inputs, etc.

It looks like a global configuration. What happens when two template libraries in the same context are both using incremental-dom and have conflicting configurations?

They are global. We could potentially create iDOM instances, each being able to specify their own configuration.

If it really is that much of an impact to allocate Arrays

I'd actually be really supportive of attribute maps (or arrays). It'll allow us to unwind the method inlining.

Speaking of, are there benchmarks we can try out?

I've been using http://jridgewell.github.io/js-repaint-perfs/incremental-dom/ locally. We should definitely look into automatic benchmark testing.

[justinfagnani]

I'm not sure I like this direction, as it would require changes to consider attributes and properties fundamentally different.

I don't understand the specific objection. The difference between attribute and properties only how they're set and read. You just need a single extra bit to flow with the name/value pair:

set(node, name, value, isProperty) {
  if (isProperty) {
    node[name] = value;
  } else {
    node.setAttribute(name, value);
}

Yes, they are different in how they are set, but whether they are "fundamentally" different I guess is for you to decide. As I mentioned I think I can already get the effect I want by wrapping primitives in objects (and making sure I unbox Strings that I want as attributes?). This means there is a code path from elementOpen to where attributes are set that I can use to force property setting, it's just ungainly and requires unfortunate wrapping.

With the current configuration step, anything from Polymers attr$ declaration to Angular's awkward [bind] syntax can be supported.

The syntax of the template doesn't really matter at all though. The attribute name in the template will be decoded to a target attribute name or property name. I don't see any reason to pass it down to incremental-dom APIs, unless names are really the only to get per-call attribute/property differentiation. Even then, there's no reason to use the template languages particular naming scheme, you could just as easily prefix attributes with something like "--attr--".

Still, going through the configuration function is a few steps to many, and requires this encoding, when it doesn't seem necessary.

idom.patch(node, function() {
  while (node = nextNode()) {
    if (node.nodeType === Node.ELEMENT_NODE) {
      idom.elementOpen(node.tagName);
      for (let attr in getAttributeNames(node)) {
        if (isProperty(attr) {
         idom.property(attr.name, attr.value);
        } else {
         idom.attribute(attr.name, attr.value);
        }
      }
      idom.elementClose(node.tagName);
  }
}.bind(this));

My question about the configuration API is what exactly is gained by putting the if(isProperty(attr)) logic somewhere else, especially somewhere global? What if the logic changes over time (as with different template syntaxes in the same context)? Should I be doing logic like this when setting the configuration:

if (mutators.attributes.__all) {
  throw "Conflicting incremental-dom usage!";
}
mutators.attributes.__all = ...;

Another way I think about this is the orthogonal attributes of attributes:

	Static	Dynamic
Attribute	elementOpen() arg 3	elementOpen() varargs
Property	none?	intercept via mutators.attributes.__all

As an API user I find that hard to grok (even if incremental-dom is a compile target, I still need to write the compiler or interpreter).

[sparhami]

It can be done, but it would require some fairly substantial changes. In order to tell what attributes have disappeared, we need a full view of the attributes/properties. Even if you aren't doing arbitrary logic in the attributes declaration like for-loops and switches, you can still end up with cases where 'disappearing' attributes need to be removed. For example:

if (condition) {
  elementVoid('div', null, null, 'attrOne', 'value');
} else {
  elementVoid('div', null, null, 'attrTwo', 'value');
}

As far as Incremental DOM is concerned, it can't tell what happened other than then attributes are now different, so it needs to strip attrOne and add attrTwo. If you want to have control with such an API you would basically be doing something along the lines of what elementOpenStart, attr and elementOpenEnd do, which currently comes at a rather large performance cost.

[jridgewell]

Yes, they are different in how they are set, but whether they are "fundamentally" different I guess is for you to decide.

I was talking more about having different code paths to support both. That we'd be splitting the API into (nodeName, key, statics, attributes, props) (or attr and prop functions) implies that attributes and properties are different, vs the current API that treats them much like the DOM does (node.id = 'test' is the same as node.setAttribute('id', 'test')).

This means there is a code path from elementOpen to where attributes are set that I can use to force property setting, it's just ungainly and requires unfortunate wrapping.

Oh, absolutely. The current property setting was intended only for passing Objects around and event listeners. It's definitely ungainly for forcing property setting.

My question about the configuration API is what exactly is gained by putting the if(isProperty(attr)) logic somewhere else...

The main downside I see is doubling up the logic for attributes and properties. Things like caching, the list of currently set (so that we can unset missing as @sparhami highlighted). Speed would also be an issue, but it wouldn't be significantly worse than if we adopted attribute maps/arrays (taking away the var_args stuff).

... especially somewhere global?

That it's global is definitely a downside, no argument there.

As an API user I find that hard to grok (even if incremental-dom is a compile target, I still need to write the compiler or interpreter).

I think I'm starting to see the issue here, thanks for the table. To iDOM, properties are just attributes. We treat Objects and functions a bit differently during setting, since no one wants those coerced to strings. But, mutators.attributes are called for both properties and attributes (properties are attributes).

	Static	Dynamic
Attribute	elementOpen() arg 3	elementOpen() varargs
Property	elementOpen() arg 3	elementOpen() varargs

Additionally, if you want to intercept static or dynamic attributes/properties, providing the appropriate mutators.attributes mutator will do the trick. Or, __all to intercept any attributes/properties.

[treshugart]

FWIW:

They are global. We could potentially create iDOM instances, each being able to specify their own configuration.

We absolutely require this be possible. See #254 (comment).

If this is something that helps achieve the goal in this issue, then 2 birds, 1 stone etc. However, it seems that can be done separately to this. Should I raise an issue for that or convert my question into an issue for that (just not sure what workflow you prefer)?