Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Actions do not execute in left-to-right order if options differ #806

Closed
zombiezen opened this issue Dec 31, 2024 · 2 comments
Closed

Actions do not execute in left-to-right order if options differ #806

zombiezen opened this issue Dec 31, 2024 · 2 comments

Comments

@zombiezen
Copy link

As of 2024-12-31, the Stimulus reference for Actions says:

When an element has more than one action for the same event, Stimulus invokes the actions from left to right in the order that their descriptors appear.

This isn't entirely accurate. If you examine the dispatcher, a separate event listener is registered for each unique sequence of options (presumably to support the options to addEventListener):

const cacheKey = this.cacheKey(eventName, eventOptions)
let eventListener = eventListenerMap.get(cacheKey)
if (!eventListener) {
eventListener = this.createEventListener(eventTarget, eventName, eventOptions)
eventListenerMap.set(cacheKey, eventListener)
}

(where cacheKey is defined like so:)

private cacheKey(eventName: string, eventOptions: any): string {
const parts = [eventName]
Object.keys(eventOptions)
.sort()
.forEach((key) => {
parts.push(`${eventOptions[key] ? "" : "!"}${key}`)
})
return parts.join(":")
}

Only bindings within a single event listener will be ordered; everything else is undefined:

get bindings(): Binding[] {
return Array.from(this.unorderedBindings).sort((left, right) => {
const leftIndex = left.index,
rightIndex = right.index
return leftIndex < rightIndex ? -1 : leftIndex > rightIndex ? 1 : 0
})
}

This may be the expected for capture, once, and passive, but I would not have expected this for stop, prevent, self, or my custom options.

I suggest two possible resolutions:

  1. Rework the internals of dispatcher so that only capture, once, and passive are part of the cache key. IMO this matches what most users would expect of the library.
  2. Document this behavior without changing the code. I would find this unfortunate and reduces the utility of the action options, but is definitely less work and has less compatibility implications.
@zombiezen
Copy link
Author

Oh, it looks like #684 is exactly the first resolution I proposed. Any chance we could get that merged?

@zombiezen
Copy link
Author

(And by extension, consider this a dupe of #718. Sorry for the noise.)

@zombiezen zombiezen closed this as not planned Won't fix, can't repro, duplicate, stale Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant