0

I'm new to node js. When a player connects I want the current list of rooms to be sent to them and then a list to be built that when an entry in that list is clicked it allows the player to join that room. For some reason the function only gets applied to the last element in the array. It doesn't matter how large it is, the player can only join the most recently added room. I added a console log statement to see if it's a server side problem but the 'test' statement does not get logged when clicking any div besides the last one added.

socket.on('rooms', function (data) {
        for (var i in data) {
            roomsListDiv.innerHTML += '<div>' + data[i].roomNumber + '<div>';
            RoomDivs[data[i].roomNumber] = (roomsListDiv.lastChild);
            RoomDivs[data[i].roomNumber].style.cursor = 'pointer';
            RoomDivs[data[i].roomNumber].onclick = function () { //join room
                socket.emit('joinRoom', { room: data[i].roomNumber });
                console.log("test");
            }
        }
    });
Andy D
  • 31
  • 4
  • 2
    Change `for(var i` to `for(let i` and try again. This issue is related to how `var` works with JavaScript closures. `let` is different, it scopes to block level. – David784 May 11 '20 at 18:00

1 Answers1

2

You need to use let or const instead of var to declare the variable i since let and const variables are scoped to the immediate enclosing block whereas variables declared using var are scoped to the enclosing function.

socket.on('rooms', function (data) {
        for (let i in data) {
            roomsListDiv.innerHTML += '<div>' + data[i].roomNumber + '<div>';
            RoomDivs[data[i].roomNumber] = (roomsListDiv.lastChild);
            RoomDivs[data[i].roomNumber].style.cursor = 'pointer';
            RoomDivs[data[i].roomNumber].onclick = function () { //join room
                socket.emit('joinRoom', { room: data[i].roomNumber });
                console.log("test");
            }
        }
    });

Using var, your code is essentially the same as if you had declared i at the top of your function, in which case its value would be updated by the loop each iteration and end up with the last assigned value, i.e. it is equivalent to the following:

socket.on('rooms', function (data) {
        var i;
        for (i in data) {
            roomsListDiv.innerHTML += '<div>' + data[i].roomNumber + '<div>';
            RoomDivs[data[i].roomNumber] = (roomsListDiv.lastChild);
            RoomDivs[data[i].roomNumber].style.cursor = 'pointer';
            RoomDivs[data[i].roomNumber].onclick = function () { //join room
                socket.emit('joinRoom', { room: data[i].roomNumber });
                console.log("test");
            }
        }
    });
Unmitigated
  • 76,500
  • 11
  • 62
  • 80
  • I tried this but the function is still only getting called for the last div in the list – Andy D May 11 '20 at 18:37
  • @AndyD Could you please provide a reproducible example of this behavior? – Unmitigated May 11 '20 at 18:44
  • @AndyD Could you please create an example that reproduces the error in JSFiddle? – Unmitigated May 11 '20 at 18:58
  • https://jsfiddle.net/bhsvopcn/2/ 'test' only gets logged when clicking the last div (103) – Andy D May 11 '20 at 19:05
  • Here is another example where I added a bind to the callback function but it still the same https://jsfiddle.net/dembofskya/s7kmLr2q/2/ – Andy D May 11 '20 at 19:12
  • @AndyD I fixed the issue. You should use `document.createElement('div')` to create your new element and use `.appendChild` to add it to the container. https://jsfiddle.net/xtawpc12/ – Unmitigated May 11 '20 at 21:46