Fix skipping of event handlers #3201
Conversation
Event handlers registered after a `once` handler were being silently skipped, causing unpredictable behaviour in tour interactions. This occurred because modifying an array during iteration is unsafe. Making a copy of the bindings array before iteration prevents these handers from being skipped.
|
@bausmeier is attempting to deploy a commit to the shipshapecode Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| trigger(event: string, ...args: any[]) { | ||
| if (!isUndefined(this.bindings) && this.bindings[event]) { | ||
| this.bindings[event]?.forEach((binding, index) => { | ||
| this.bindings[event]?.slice().forEach((binding, index) => { |
There was a problem hiding this comment.
Another approach I considered was to iterate the bindings in reverse order with a for loop. This would allow us to avoid making a copy of the array, but could have an observable impact for existing users of the package. I thought it would be best to avoid what could be considered a breaking change.
There was a problem hiding this comment.
@bausmeier what if instead of creating a copy, we made a new array like const listenersToRemove = [] and then pushed the ones to remove into that, then after the forEach is done remove them all?
|
@bausmeier sorry for the delay in reviewing this! Could you please explain the exact problem a bit more? |
|
@RobbieTheWagner Sure thing. Sorry I didn't explain more clearly in the first place. In our project, we need to listen for the same event in multiple places (analytics and business logic), and in the particular case where this problem arises we're listening for the As an illustrative example, we call Hope that clarifies things. If you need an example to reproduce this I can try and put something together, but I was able to write a failing test so didn't think that was necessary. |
2 participants
Event handlers registered after a
oncehandler were being silently skipped, causing unpredictable behaviour in tour interactions. This occurred because modifying an array during iteration is unsafe.Making a copy of the bindings array before iteration prevents these handers from being skipped.