webcomponents: unnecessary listeners oprations

[WebReflection]

Describe the bug The update() logic remove and add the very same listener for no gain/reason whatsoever.

To Reproduce Steps to reproduce the behavior:

1. visit this demo page
2. note the following code as output:

  update() {
    this.querySelectorAll("[data-name='input-1']").forEach((el) => {
      el.value = this.state.name; 
       // these two operations are unnecessary because the listener is the same
       // and the DOM would not add twice the same reference/listener
      el.removeEventListener("input", this.onInput1Input);
      el.addEventListener("input", this.onInput1Input);
    });
  }

Expected behavior Adding twice the very same listener doesn't mean the listener is triggered twice. Removing and adding the same listener one line after the other results in bloated DOM operations for no gain reason, or benefit, it's just more code, and slower.

[steve8708]

There is a reason for this that isn’t well represented in this example. More just adding this note for if anyone wants to tackle this lmk and I can give more detail of the things to know before making changes so the cases where this is needed can be addressed

[steve8708]

To clarify though I do agree in this example those are not needed. The necessity comes in when you use loops in the dom, in the current webcomponent output we don’t do diffing and instead wipe and recreate nodes, which creates the need to rebind event listeners

the two obvious solutions here would be to implement diffing directly, but that would defeat the purpose a bit of not having additional dependent code, or use the Stencil output which has all of that built in for you.

That or the best option may be to update the logic here to be smarter - don’t generate re-binding code for any node in the tree unless it has a parent that is a For component. This could be done pretty straightforwardly, and contributions are more than welcome

[WebReflection]

I am not sure I understand what re-bindings mean here, but if it's about the runtime assigned methods in the constructor (slow) I'd like to remind you there are better ways to don't bloat the RAM and use just the component itself as event listener, without needing to bloat the constructor neither, sharing same prototype, example:

/**
 * Usage:
 *
 *  <my-component></my-component>
 *
 */
class MyComponent extends HTMLElement {
  constructor() {
    super().state = { name: "Steve" };
  }

  handleEvent(event) {
    this["@" + event.type](event);
  }

  ['@input'](event) {
    this.state.name = event.target.value;
    this.update();
  }

  connectedCallback() {
    this.innerHTML = `
      <div>
        <input class="input" data-name="input-1" />

        Hello! I can run in React, Vue, Solid, or Liquid!
      </div>
      <style>
        .input {
          color: red;
        }
      </style>`;
    this.update();
  }

  update() {
    this.querySelectorAll("[data-name='input-1']").forEach((el) => {
      el.value = this.state.name;
      el.addEventListener("input", this);
    });
  }
}

customElements.define("my-component", MyComponent);

Maybe this post helps more understanding how to benefit from this handleEvent pattern and also help solving bloat and performance when webcomponents are used as target?

[steve8708]

Ah, this example explains things really well and is a great idea. Pull request would be very welcomed!

[WebReflection]

I'd love to contribute but I need everyone to be aligned around this topic/issue as that would influence everything else, in terms of outcome, and this fix itself wouldn't solve, or work, if that issue is not addressed.

[steve8708]

That would be amazing. Just replied on the other topic, I believe that one is a misunderstanding (just something that wraps the component for our Builder.io visual editing preview to work, and not something that affects the Mitosis output code), but let me know over there if I am wrong