[Feature Request] Align how _inputNode is checked in Input and Textarea

[amielucha]

While I was reviewing the differences between the implementation of property changing in input and textarea components, I noticed there's a slightly different approach to how this._inputNode value is checked. In Textarea there's an additional variable native.

const native = this._inputNode;
if (native) {
  native.readOnly = this.readOnly;
}

Is the check necessary? In which scenario native would be undefined? If it's needed, shouldn't it be also applied to updated() in Input?

updated(changedProps) {
  super.updated(changedProps);
  if (changedProps.has('type')) {
    this._inputNode.type = this.type;
  }
  if (changedProps.has('placeholder')) {
    this._inputNode.placeholder = this.placeholder;
  }
}

[daKmoR]

the difference is that

const native = this._inputNode;
if (native) {
  native.readOnly = this.readOnly;
}

get's executed sync in

lion/packages/input/src/LionInput.js

Line 60 in 54f5853

this.__delegateReadOnly();

so this can get executed before the dom node is available (maybe it gets rendered?)

---

In comparison updated always gets executed async AFTER a render - therefore, in this case, we can be sure that the element is available.

does my explanation makes sense? 😅

[amielucha]

Thanks for the clarification. This raises additional question though.

If there is no need for the check in updated, could this

lion/packages/textarea/src/LionTextarea.js

Lines 68 to 73 in 60284d7

	if (changedProperties.has('rows')) {
	const native = this._inputNode;
	if (native) {
	native.rows = this.rows;
	}
	}

knowing that it's inside updated be simplified as follows?

if (changedProperties.has('rows')) {
  this._inputNode.rows = this.rows;
}

If it's the case, I could follow up with another PR after the addition of placeholders in Textarea.

[jorenbroekema]

Personally I wouldn't mind if we can get rid of redundant guards, so feel free to send a PR, if the tests pass I would be okay with merging, going by daKmoR's logic it should be fine. If it fails, it might be an indication something else is wrong.

[jorenbroekema]

@daKmoR I looked into this more and my proposal would be the following:

• Have a protected delegation method in LionInput:

_delegatePropToInputNode(propName) {
  this._inputNode[propName] = this[propName];
}

• Call it in connectedCallback() for synchronously delegating on initial (because slotmixin appends slots in connectedCallback, so then we do it immediately when the node is available)
• Call it in updated() if changedProperties has propName, for imperatively updating a prop and making sure it's synced down to the input node as well.

Then we make sure it's consistent everywhere, for rows, autocomplete, placeholder, type, readonly etc. etc.

Thoughts?

[jorenbroekema]

Closing as enhancement with votes needed. If someone wants to push for this, feel free! Or vote if you think it's important by 👍 'ing.

[jorenbroekema]

We have strict types now in Lion, and inputNode is now always strictly typed as not being undefined, so that kinda solves it and removes the need for the guard 👍