telehash / telehash-js

telehash javascript module for node.js and browserify
http://telehash.org
MIT License
301 stars 51 forks source link

Do not add transports to default extensions #23

Open Kagami opened 9 years ago

Kagami commented 9 years ago

By running this simple mesh node creates 3 listening sockets: two for TCP and one for UDP:

var th = require("telehash");

th.generate(function(err, id) {
  th.mesh({id: id}, function(err, mesh) {
    console.log(mesh);
  });
});

That's because of the transport extensions added by default on module load.

I could unload them by calling:

delete th.extensions.udp4;
delete th.extensions.tcp4;
delete th.extensions.http;

but it looks ugly as for me. (Also little bug: extensions should be an object.) Should it be the option to disable default extensions without manually removing them?

quartzjer commented 9 years ago

:+1: this should be easier and more obvious, good feedback. If you take a look at the tests you'll see that you can pass in a fixed list of extensions when you create a mesh that will override, is that helpful?

Kagami commented 9 years ago

I think it's good to always have all default channel extensions (from the ext/ directory). But the one who uses the library should choose the needed transports and add them manually.

Also how about more advanced unregister/config API for extensions? E.g. to allow users to add/remove extensions by type (channel, transport, cipher), option to disable all extensions and so on.

Another note: I'm note sure it's generally good idea to listen on some port by just adding the transport. Or maybe all transports should be splitted into the client and server parts (like http-client and http-server) so the users can choose essential functionality?

bachp commented 9 years ago

I think it would make sense to have something like app.use from express.js where the user can register all required extension.

Only the registered extensions should be available. Same for ciphersets and other things. This would also help in some deployment scenarions (sodium isn't running everywhere).

quartzjer commented 9 years ago

Yes, I very much want to move to this model here too, it's near the top of my TODO list now so it should happen soon :)