Input `value` set as attribute about incremental-dom HOT 15 CLOSED

I've actually ran into the same problem as @peterwmwong initially mentioned, and I've been thinking about it.

My initial approach was just to change the setAttribute of applyAttr to setting the property. However this caused problems when writing at an <input>: as soon as you change the value property, the cursor position would be altered. It also creates problems with the actual DOM representation (attribute nodes not being changed).

After a few iterations, I've came up with this solution, which I think it's easier to maintain than having a complete map of attributes and their way of being updated. Basically we first update the attribute, and after, if the corresponding property hasn't been updated, we change it as well.

However, I'm pretty sure it has some caveats, and might not fit all cases, but just to give some other options on it. 😃

from incremental-dom.

Referenced code (this one) does not cause issues, because, when writing, the value property is altered by the browser, so when checking its value (in line 53), both the input.value and the new value will match, and the property will not be changed (which is what causes the losing focus / unaligning).

On the other side, if you change the value externally, and then you patch, the === will not match, and then the code will assign the new property value to value, thus reflecting the change on the DOM. In other words, we just change attributes and properties only if something else didn't already change it.

Although the code supports mutators now, I would say that the fix should be included in the core of the library, rather than as an additional "plugin"; mainly because I think this is the default behavior you expect. mutators seems for me something more with a "client" perspective, rather than a "library" thing (but maybe I'm wrong).

from incremental-dom.

React has a lot of code to support these attribute vs property cases, particularly /src/renderers/dom/shared/HTMLDOMPropertyConfig.js#L44-L193. Might be worth porting over?

from incremental-dom.

@jridgewell sounds good. Then I can use it too for jsx html/dom renderers, so one JSX input could be transformed to equivalent (in terms of end tree) HTML, DOM or Incremental DOM output.

from incremental-dom.

We probably can has separate npm package so all can depend on it and this way almost all tests will be one place, + same update for everyone.

from incremental-dom.

Maybe we could test for property existance on the element and read/write from/to this, otherwise use get/setAttribute...? Related to #57

from incremental-dom.

I am curious why setting for example attr('_test', 'val') should be attribute instead of property? I believe setting/accessing property is faster than the attribute.

from incremental-dom.

See #45 for a bit more context.

from incremental-dom.

For value and checked (and any others?), I don't think setting those as a property would be too bad, although it introduces a slight inconsistency. We would also need to introduce a defaultValue/defaultChecked and anyone translating templates would need to know that value should map to defaultValue etc. when compiling.

Setting arbitrary things as properties would definitely be faster, but there already exists a lot of markup (and logic relying on that markup) that uses non data- attributes on non-custom elements. so it is a no-go, at least for now. It would be possible to have some configuration options, but that makes things quite chaotic in the long run.

from incremental-dom.

@sparhami configuration options sounds good, exactly that I thought. Also, about data- attributes--I believe you should treat any dashed attribute as a attribute anyway because there is not useful case to treat them as properties. Although, there are cases for simple names. Consider this example with JSX:

<div class="list-item"
  data-bind="item"
  itemData={ item.data }
>
  ...
</div>

In this case it would be good to translate data-bind attribute to real attribute since it has dash, and translate itemData to element's property. I know that objects already translates to props, but that might be itemIndex with number value, or something other.

from incremental-dom.

We could deliver a dedicated props object together with staticattrs / attrs and allow for properties to be set only through this. All other keyvalues would then be set as attributes. This would offload responsibility to the developer or transformer logic, but would also simplify the whole process of knowing what's a property and what is an attribute.

from incremental-dom.

So any updates on this? @jridgewell @sparhami

from incremental-dom.

as soon as you change the value property, the cursor position would be altered

Doesn't your solution also cause issues?

With the current codebase, you can set mutators.attributes[symbols.all] to do this:

iDOM.mutators.attributes[iDOM.symbols.all] = function(el, name, value) {
    var type = typeof value;
    var propertyName;

    if (PROPERTY_MAP.hasOwnProperty(name)) {
      propertyName = PROPERTY_MAP[name];
    } else {
      propertyName = name;
    }

    if (value === undefined) {
      el.removeAttribute(name);
    } else if ((typeof value !== 'function') && (typeof value !== 'object')) {
      el.setAttribute(name, value);
    }

    // If, after changing the attribute, the corresponding property is not updated,
    // also update it.
    if (el[propertyName] !== value) {
      el[propertyName] = value;
    }
};

from incremental-dom.

Also, iDOM.mutators.attributes is a mouthful. Want to change it to just iDOM.attributes (or something else) since we're not putting the DOM notifications in the same namespace?

from incremental-dom.

Also, iDOM.mutators.attributes is a mouthful. Want to change it to just iDOM.attributes (or something else) since we're not putting the DOM notifications in the same namespace?

Sounds good to me.

from incremental-dom.