Skip to content

Commit

Permalink
Ensure all event listeners are called (phoenixframework#3576)
Browse files Browse the repository at this point in the history
* Ensure all event listeners are called

Fixes: phoenixframework#3572

If we have 2 different listeners and one of them removes the listener
in its body then the second listener is never called:

```javascript
var ref = channel.on("event", () => {
 channel.off("event", ref);
});

channel.on("event", () => {
 // never called
});
```

This is because the listeners array is used by reference and all the
changes to it are visible before all the listeners are called. Thus, if
one of the listeners is removed then since `i` has not change the next
in line listener is skipped.

To fix this copy the array of listeners before calling them. This
approach is used in many libraries from what I saw ([1], [2]).

[1]: https://github.com/component/emitter/blob/6bd7817e8a444cb16e8abdf7dd2d7f04d5ca3dc8/index.js#L143
[2]: https://github.com/sindresorhus/emittery/blob/3334cff4daf83bfcf7054d35edc51957e3f70f8d/index.js#L220

* Update assets/js/phoenix.js

* Update assets/js/phoenix.js

Co-authored-by: Chris McCord <[email protected]>
  • Loading branch information
ostap0207 and chrismccord authored Mar 19, 2020
1 parent 3948cf2 commit 90e082e
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
7 changes: 4 additions & 3 deletions assets/js/phoenix.js
Original file line number Diff line number Diff line change
Expand Up @@ -617,9 +617,10 @@ export class Channel {
let handledPayload = this.onMessage(event, payload, ref, joinRef)
if(payload && !handledPayload){ throw new Error("channel onMessage callbacks must return the payload, modified or unmodified") }

for (let i = 0; i < this.bindings.length; i++) {
const bind = this.bindings[i]
if(bind.event !== event){ continue }
let eventBindings = this.bindings.filter(bind => bind.event === event)

for (let i = 0; i < eventBindings.length; i++) {
let bind = eventBindings[i]
bind.callback(handledPayload, ref, joinRef || this.joinRef())
}
}
Expand Down
13 changes: 13 additions & 0 deletions assets/test/channel_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -809,6 +809,19 @@ describe("with transport", () => {
const ref2 = channel.on("event2", () => 0)
assert.equal(ref1 + 1, ref2)
})

it("calls all callbacks for event if they modified during event processing", () => {
const spy = sinon.spy()

const ref = channel.on("event", () => {
channel.off("event", ref)
})
channel.on("event", spy)

channel.trigger("event", {}, defaultRef)

assert.ok(spy.called)
})
})

describe("off", () => {
Expand Down

0 comments on commit 90e082e

Please sign in to comment.