2

I have a issue with a callback function's variable losing its scope

. I have the following 2 objects in an array(simplified down to show the problem)

const search = [{socket: new WebSocket('ws://live.trade/123')}, 
                {socket: new WebSocket('ws://live.trade/xyz')}];

I then do a forEach on them in an attempt to log the sockets url once the socket is open.

search.forEach(function(element){
    element.socket.on('open', function open() {
      console.log(element.socket.url);
    });
});

*actual output*
ws://live.trade/xyz
ws://live.trade/xyz

*expected*
ws://live.trade/123
ws://live.trade/xyz

I feel like the reason is that when the function open() runs element is not in scope and it just uses whatever was last there(being the ws://live.trade/xyz). Is this correct? And finally what would be the way to go about fixing this? The real use for this is when the socket is opened I need to send version data to the server via the socket that called it... Im going to have many sockets in reality and dont want to write a "socket.on('open'...)" for each individual one.

Any suggestions?

Thanks so much!

Brent Aureli
  • 463
  • 6
  • 21
  • You are correct, `element.socket.url` will show you its last value. I think you would need to use `this` keyword. Not familiar with sockets much, but something like `this.url` probably? – Alexander Apr 27 '18 at 06:23
  • 1
    *"Is this correct?"* No. JavaScript has lexical scope. Which library are you using? I assume you are using one because native `WebSocket` instances don't have an `on` method. – Felix Kling Apr 27 '18 at 07:13
  • I am using "ws: a NodeJS websocket library" https://github.com/websockets/ws – Brent Aureli Apr 27 '18 at 15:34

1 Answers1

1

Your callback (i.e. in socket.on()) uses the element variable of the forEach(). From your actual result, it could mean that:

1. Scope issue

The variable could be "overriden" over the iterations because of javascript/node scope mechanism, it is not the case with the code you gave but it is a common issue working with scopes inside loops.

in this case you would have to do :

search.forEach(function(element){
    element.socket.on('open', function(){
      console.log(this.socket.url);
    }.bind(element));
});

Explanation:

  • bind() passes its first argument (the element variable in this case) to the anonymous function as this inside it.
  • Inside your callback function, this.socket is equivalent to element.socket at the appropriate iteration

[UPDATE] after pointed out by Felix Kling, forEach() provides a scope for each element.

See bind for more details.

2. Dependency issue

If it is not a scope issue, it could mean that the library you are using does not work as expected. Maybe, try to log element inside the forEach() to check if the WebSocket objects are what you expect.

edit

  • For further reading on node/js loops and reference: see here
  • For node/js closures in general: see here
xdrm
  • 399
  • 2
  • 10
  • 1
    *"this variable is overriden over the iterations because of javascript/node scope mechanism."* That's incorrect. Each function call has its own scope. Proof: https://jsfiddle.net/eeqbgmh9/. – Felix Kling Apr 27 '18 at 07:08
  • You are right ! I will edit my answer not to be so "imperative". I answered without trying out from my experience that is to be improved in this case .. I'm new to stackoverflow, do I have to delete or edit my answer ? – xdrm Apr 27 '18 at 07:28
  • Bind the element object to the inner function did the trick! I will have do some more reading on the bind function for a better understanding! Thanks soo much for the help and also thanks to @FelixKling – Brent Aureli Apr 27 '18 at 16:18
  • 1
    @BrentAureli: While it may work, it's not possible that the code in your question has this problem, unless `WebSocket` is broken (but the code on GH looks fine) or *you* are somehow changing the objects in the array. Lets try something simpler. What if you did this: `search.forEach(function(element){ var socket = element.socket; socket.on('open', function open() { console.log(socket.url); }); });` – Felix Kling Apr 27 '18 at 16:24
  • Editing your answer to remove the incorrect statements would be a start. You can still keep your solution as possible workaround. – Felix Kling Apr 27 '18 at 16:25
  • @FelixKling I believe you are correct. That code also works. Which doesn't make sense, I believe you are correct that in my attempt to simplify the question on GH i may have removed some critical type in which I may have adjusted the object. During the refactoring with bind I must have fixed it incidentally. Its likely I should remove this question, as it was probably my fault originally though the answers did assist in resolving it. Thanks! – Brent Aureli Apr 28 '18 at 02:24