Always clean up event listener on binding disconnect

src/core/binding_observer.ts

@@ -7,7 +7,7 @@ import { Token, ValueListObserver, ValueListObserverDelegate } from "../mutation
 
 export interface BindingObserverDelegate extends ErrorHandler {
   bindingConnected(binding: Binding): void
-  bindingDisconnected(binding: Binding, clearEventListeners?: boolean): void
+  bindingDisconnected(binding: Binding): void
 }
 
 export class BindingObserver implements ValueListObserverDelegate<Action> {
@@ -72,7 +72,7 @@ export class BindingObserver implements ValueListObserverDelegate<Action> {
   }
 
   private disconnectAllActions() {
-    this.bindings.forEach((binding) => this.delegate.bindingDisconnected(binding, true))
+    this.bindings.forEach((binding) => this.delegate.bindingDisconnected(binding))
     this.bindingsByAction.clear()
   }
 

src/core/dispatcher.ts

@@ -41,9 +41,9 @@ export class Dispatcher implements BindingObserverDelegate {
     this.fetchEventListenerForBinding(binding).bindingConnected(binding)
   }
 
-  bindingDisconnected(binding: Binding, clearEventListeners = false) {
+  bindingDisconnected(binding: Binding) {
     this.fetchEventListenerForBinding(binding).bindingDisconnected(binding)
-    if (clearEventListeners) this.clearEventListenersForBinding(binding)
+    this.clearEventListenersForBinding(binding)
   }
 
   // Error handling

src/tests/modules/core/event_options_tests.ts

@@ -62,15 +62,36 @@ export default class EventOptionsTests extends LogControllerTestCase {
     this.assertActions({ name: "log", eventType: "keydown" })
   }
 
-  async "test edge case when updating action list with setAttribute preserves once history"() {
+  async "test edge case when updating action list with setAttribute loses once history if events are prepended to it"() {
     await this.setAction(this.buttonElement, "c#log:once")
 
     await this.triggerEvent(this.buttonElement, "click")
     await this.triggerEvent(this.buttonElement, "click")
 
-    //modify with a setAttribute and c#log should not be called anyhow
+    //modify with a setAttribute and c#log should be called once again
     await this.setAction(this.buttonElement, "c#log2 c#log:once d#log")
     await this.triggerEvent(this.buttonElement, "click")
+    await this.triggerEvent(this.buttonElement, "click")
+
+    this.assertActions(
+      { name: "log", identifier: "c" },
+      { name: "log2", identifier: "c" },
+      { name: "log", identifier: "d" },
+      { name: "log", identifier: "c" },
+      { name: "log2", identifier: "c" },
+      { name: "log", identifier: "d" }
+    )
+  }
+
+  async "test edge case when updating action list with setAttribute keeps once history if action keeps the same index"() {
+    await this.setAction(this.buttonElement, "c#log:once")
+
+    await this.triggerEvent(this.buttonElement, "click")
+    await this.triggerEvent(this.buttonElement, "click")
+
+    //modify with a setAttribute and c#log should be called once again
+    await this.setAction(this.buttonElement, "c#log:once c#log2 d#log")
+    await this.triggerEvent(this.buttonElement, "click")
 
     this.assertActions(
       { name: "log", identifier: "c" },

src/tests/modules/core/memory_tests.ts

@@ -9,8 +9,8 @@ export default class MemoryTests extends ControllerTestCase() {
 
   fixtureHTML = `
     <div data-controller="${this.identifier}">
-      <button data-action="${this.identifier}#doLog">Log</button>
-      <button data-action="${this.identifier}#doAlert">Alert</button>
+      <button id="button1" data-action="${this.identifier}#doLog">Log</button>
+      <button id="button2" data-action="${this.identifier}#doAlert ${this.identifier}#log2">Alert</button>
     </div>
   `
 
@@ -19,4 +19,22 @@ export default class MemoryTests extends ControllerTestCase() {
     await this.fixtureElement.removeChild(this.controllerElement)
     this.assert.equal(this.application.dispatcher.eventListeners.length, 0)
   }
+
+  async "test removing a button clears dangling eventListeners"() {
+    this.assert.equal(this.application.dispatcher.eventListeners.length, 2)
+
+    const button = this.findElement("#button1")
+    await this.remove(button)
+
+    this.assert.equal(this.application.dispatcher.eventListeners.length, 1)
+  }
+
+  async "test removing a button clears dangling eventListeners with multiple actions"() {
+    this.assert.equal(this.application.dispatcher.eventListeners.length, 2)
+
+    const button = this.findElement("#button2")
+    await this.remove(button)
+
+    this.assert.equal(this.application.dispatcher.eventListeners.length, 1)
+  }
 }