nidium/Nidium

Socket.prototype.onread fired incorrectly

cookiengineer opened this issue · 2 comments

Currently, if you start a server, it will always receive data via onread() because of JSSocket.h#L79.

This is a bit confusing, is this a wanted API? It will eventually lead to users having to create a cache for both their wrappers and the nidium sockets, just to answer to the corresponding socket correctly.

Example 1 (current behaviour)

let server = new Socket('127.0.0.1', 1234).listen('tcp');

server.onaccept = socket => console.log(socket.ip, socket.port);
server.onread = (socket, data) => console.log('server.onread always fired', data);

Example 2 (expected behaviour)

let server = new Socket('127.0.0.1', 1234).listen('tcp');

server.onaccept = function(socket) {

    socket.onread = function(data) {
        // XXX: Never actually fired
        console.log('this is a much better API, per-client onread', data);
    };

};

server.onread = (socket, data) => console.log('server.onread always fired', data);

The issue with the "expected behavior" your described is that it creates a new function per client.
This could be an issue when having to deal with a lot of objects.

Anyway, we can still do it and keep both API (server.onread + per-socket onread) ?
e.g. don't fire server.onread if socket has onread defined?

@paraboul Yep, that's what I was thinking.