Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Socket.prototype.onread fired incorrectly #76

Open
cookiengineer opened this issue Apr 19, 2017 · 2 comments
Open

Socket.prototype.onread fired incorrectly #76

cookiengineer opened this issue Apr 19, 2017 · 2 comments

Comments

@cookiengineer
Copy link
Contributor

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);
@paraboul
Copy link
Member

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?

@cookiengineer
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants