Duplicate event handlers being added to elements with ids during LiveView update.

[cthulahoops]

I've been running into a weird bug with event handlers being added more than once to elements when using AlpineJS with LiveView.

This is a fairly minimal demonstration:

<script>
function add_message(dispatch) {
  event.preventDefault()
  let message = document.getElementById("message").value;
  dispatch('add_message', {message: message});
}
</script>

<ul id="to-input-div" x-data="{}" phx-hook="PushEvent" phx-push-event="add_message">
<%= for message <- @messages do %>
  <li><%= message %></li>
<% end %>
  <input id="message" x-on:keydown.enter="add_message($dispatch)">
  <button x-on:click="add_message($dispatch)">Add (no id)</button>
  <button id="unreferenced" x-on:click="add_message($dispatch)">Add (with id)</button>
</ul>

When you enter a message and hit enter it gets added once, and a second copy of the event handler is added. The second message is added twice, the third four times, the fourth eight.... Clicking the button with an id does the same thing, but clicking the button without id only adds it once. (Checking registered event handlers confirms that an event handler has been added for every message added to the list.)

This is the phx-hook:

Hooks.PushEvent = {
  mounted() {
    const eventName = this.el.attributes['phx-push-event'].value;
    this.el.addEventListener(eventName, (event) => { this.pushEvent(eventName, event.detail) })
  }
}

And I have the callback calling Alpine.clone from onBeforeElUpdated.

(Running alpinejs 2.8.1 and phoenix_live_view 0.15.4).

[cthulahoops]

The difference in behaviour occurs because morphdom uses the node's id to identify nodes that should be moved rather than deleted and reinserted. I think the bug occurs because element moves aren't distinguished from element creations.

With this in mind, I can write an version that triggers the issue without liveview:

<div id="to-input-div" x-data="{'x': 0}">
  <span x-text="x">0</span>
  <button id="a" x-on:click="x += 1; $el.appendChild(document.getElementById('a'))">A</button>
  <button id="b" x-on:click="x += 1; $el.appendChild(document.getElementById('b'))">B</button>
  <button id="c" x-on:click="x += 1; $el.appendChild(document.getElementById('c'))">C</button>
</div>

I expect that clicking a button will increment the counter and move that button to the end. When the buttons get moved duplicate event handlers get attached and the counter starts to go up in steps of more than one.

[lawik]

I think I'm seeing something very similar. Going to see if the PR solves my issue when it goes through.

[chriischambers]

Hi guys. So this issue still exists when your event is attached to a node within the node you are moving. I have updated the example to demonstrate this :

<div id="to-input-div" x-data="{'x': 0}">
    <span x-text="x">0</span>
    <div id="a"><button x-on:click="x += 1; $el.appendChild(document.getElementById('a'))">A</button></div>
    <div id="b"><button x-on:click="x += 1; $el.appendChild(document.getElementById('b'))">B</button></div>
    <div id="c"><button x-on:click="x += 1; $el.appendChild(document.getElementById('c'))">C</button></div>
</div>

[HugoDF]

Hi guys. So this issue still exists when your event is attached to a node within the node you are moving. I have updated the example to demonstrate this :

<div id="to-input-div" x-data="{'x': 0}">
    <span x-text="x">0</span>
    <div id="a"><button x-on:click="x += 1; $el.appendChild(document.getElementById('a'))">A</button></div>
    <div id="b"><button x-on:click="x += 1; $el.appendChild(document.getElementById('b'))">B</button></div>
    <div id="c"><button x-on:click="x += 1; $el.appendChild(document.getElementById('c'))">C</button></div>
</div>

Are you interested in digging into the issue? Even if you submit a PR that only updates the relevant test to fail, it'll probably make it easier for someone else to solve

Couple of other notes:

• I don't think the PR that fixes this issue has been released yet (@calebporzio correct me if I'm wrong)
• here's the relevant test https://github.com/alpinejs/alpine/blob/master/test/on.spec.js#L560-L591

[chriischambers]

@HugoDF - I can confirm that the previous fix has not yet been released.

As requested I have opened up a PR with a failing test here - #1265

It would appear that the previous solution tracks in the listenForNewElementsToInitialize method a __x_node_add_count value but this is only being tracked on the parent node. Moving this tracking to the walkAndSkipNestedComponents loop and storing it against the correct node does indeed solve the issue and make my test pass but then this causes several other tests in the suite to fail. At this point I have ran out of time to delve deeper into this.