update code as per Sam and Javan reviews

docs/reference/actions.md

@@ -36,12 +36,11 @@ An action is a connection between:
 
 ## Descriptors
 
-The `data-action` value `click->gallery#next:once` is called an _action descriptor_. In this descriptor:
+The `data-action` value `click->gallery#next` is called an _action descriptor_. In this descriptor:
 
 * `click` is the name of the DOM event to listen for
 * `gallery` is the controller identifier
 * `next` is the name of the method to invoke
-* `once` is the event listener option
 
 ### Event Shorthand
 
@@ -84,13 +83,14 @@ You can append `@window` or `@document` to the event name in an action descripto
 
 Sometimes you may need to pass [additional options](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Parameters) to the Event Listener attached to the action. 
 
-Options are set by adding one of the allowed token to the end of the action descriptor. 
-The token is the name of the option with an exclamation mark `!` to negate a value. 
-Options are separated from the method by a column `:`
+* Options are set by adding one of the allowed token to the end of the action descriptor. 
+* The token is the name of the option with an exclamation mark `!` to negate its value.
+* Options are separated from the method by a column `:`
 
 **The followng option tokens are allowed:**
+
 Tokens        | EventListener option 
-------------- | ------------------------- 
+------------- | ---------------------
 `capture`     | `{ capture: true }`
 `once`        | `{ once: true }`
 `passive`     | `{ passive: true }`

packages/@stimulus/core/src/action_descriptor.ts

@@ -7,7 +7,7 @@ export interface ActionDescriptor {
 }
 
 // capture nos.:            12   23 4               43   1 5   56 7      768 9  98
-const descriptorPattern = /^((.+?)(@(window|document))?->)?(.+?)(#([^:]+?))(:(.+))?$/;
+const descriptorPattern = /^((.+?)(@(window|document))?->)?(.+?)(#([^:]+?))(:(.+))?$/
 
 export function parseDescriptorString(descriptorString: string): Partial<ActionDescriptor> {
   const source = descriptorString.trim()
@@ -16,10 +16,10 @@ export function parseDescriptorString(descriptorString: string): Partial<ActionD
   return {
     eventTarget:  parseEventTarget(matches[4]),
     eventName:    matches[2],
-    identifier:   matches[5],
-    methodName:   matches[7],
     eventOptions: parseEventOptionTokens(matches[9]),
-  };
+    identifier:   matches[5],
+    methodName:   matches[7]
+  }
 }
 
 function parseEventTarget(eventTargetName: string): EventTarget | undefined {
@@ -30,15 +30,21 @@ function parseEventTarget(eventTargetName: string): EventTarget | undefined {
   }
 }
 
+// function parseEventOptions(eventOptions: string = ""): AddEventListenerOptions {
+//   return eventOptions.split(":").reduce((options, token) =>
+//     Object.assign(options, { [token.replace(/^!/, "")]: !token.startsWith("!") })
+//     , {})
+// }
+
 function parseEventOptionTokens(eventOptionTokens: string = ""): AddEventListenerOptions {
   const allowedTokens = ["passive", "!passive", "once", "capture"];
-  const tokens = eventOptionTokens.split(":").filter( option => allowedTokens.indexOf(option) !== -1)
+  const tokens = eventOptionTokens.split(":").filter(option => allowedTokens.indexOf(option) !== -1)
   const regexBoolean = /^[^!]{1}.+$/; // ! negates a token
 
   return tokens.reduce(
     (result, option) => ({
       ...result,
-      [option.replace(/!/,"")]: regexBoolean.test(option)
+      [option.replace(/!/, "")]: regexBoolean.test(option)
     }),
     {} as AddEventListenerOptions
   )

packages/@stimulus/core/src/dispatcher.ts

@@ -58,11 +58,12 @@ export class Dispatcher implements BindingObserverDelegate {
 
   private fetchEventListener(eventTarget: EventTarget, eventName: string, eventOptions: AddEventListenerOptions): EventListener {
     const eventListenerMap = this.fetchEventListenerMapForEventTarget(eventTarget)
-    let eventListener = eventListenerMap.get(JSON.stringify({ eventName, ...eventOptions}))
+    const cacheKey = this.cacheKey(eventName, eventOptions)
+    let eventListener = eventListenerMap.get(cacheKey)
     if (!eventListener) {
       eventListener = this.createEventListener(eventTarget, eventName, eventOptions)
-      eventListenerMap.set(JSON.stringify({ eventName, ...eventOptions }), eventListener)
-    } 
+      eventListenerMap.set(cacheKey, eventListener)
+    }
     return eventListener
   }
 
@@ -82,4 +83,8 @@ export class Dispatcher implements BindingObserverDelegate {
     }
     return eventListenerMap
   }
+
+  private cacheKey(eventName: string, eventOptions: any): string {
+    return eventName + JSON.stringify(Object.keys(eventOptions).sort().map(key => [key, eventOptions[key]]))
+  }
 }

packages/@stimulus/core/src/event_listener.ts

@@ -6,10 +6,10 @@ export class EventListener implements EventListenerObject {
   readonly eventOptions: AddEventListenerOptions | false
   private unorderedBindings: Set<Binding>
 
-  constructor( eventTarget: EventTarget, eventName: string, eventOptions: AddEventListenerOptions) {
+  constructor(eventTarget: EventTarget, eventName: string, eventOptions: AddEventListenerOptions) {
     this.eventTarget = eventTarget
     this.eventName = eventName
-    this.eventOptions = this.convertEventOptions(eventOptions)
+    this.eventOptions = eventOptions
     this.unorderedBindings = new Set()
   }
 
@@ -18,7 +18,7 @@ export class EventListener implements EventListenerObject {
   }
 
   disconnect() {
-    this.eventTarget.removeEventListener(this.eventName, this, false)
+    this.eventTarget.removeEventListener(this.eventName, this, this.eventOptions)
   }
 
   // Binding observer delegate
@@ -51,13 +51,6 @@ export class EventListener implements EventListenerObject {
     })
   }
 
-  private convertEventOptions(eventOptions: AddEventListenerOptions ): AddEventListenerOptions | false {
-    if (Object.keys(eventOptions).length === 0) {
-      return false
-    } else {
-      return eventOptions
-    }
-  }
 }
 
 function extendEvent(event: Event) {