Skip to content

net: refactor net module to module.exports#11698

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.

Sign up for GitHub

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

Closed
claudiorodriguez wants to merge 1 commit into from
Closed

net: refactor net module to module.exports #11698

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 34 additions & 32 deletions lib/net.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -62,9 +62,9 @@ function isPipeName(s){
return typeof s === 'string' && toNumber(s) === false;
}

exports.createServer = function(options, connectionListener){
function createServer(options, connectionListener){
return new Server(options, connectionListener);
};
}


// Target API:
Expand All@@ -79,7 +79,7 @@ exports.createServer = function(options, connectionListener){
// connect(port, [host], [cb])
// connect(path, [cb]);
//
exports.connect = exports.createConnection = function(){
function connect(){
Copy link
Contributor

@mscdexmscdexMar 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we just called this createConnection() and added the connect: createConnection alias in the export list. This way we won't have to rename connect to internalConnect everywhere.

Copy link
ContributorAuthor

@claudiorodriguezclaudiorodriguezMar 5, 2017•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd still have a "connect" in the exports (i.e. part of the module's API) and a function named "connect" in the code that is not the same as the export, which is still IMHO confusing. Also there's no change in the public API, no need to rename anything outside of this module. Also references inside the module to a "connect" function that is not the same as the export are confusing.
internalConnect might not be the best name though, I admit.

Copy link
Member

@joyeecheungjoyeecheungMar 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

internalConnect does make grepping easier.

const args = new Array(arguments.length);
for (var i = 0; i < arguments.length; i++)
args[i] = arguments[i];
Expand All@@ -95,7 +95,7 @@ exports.connect = exports.createConnection = function(){
}

return Socket.prototype.connect.call(socket, options, cb);
};
}


// Returns an array [options, cb], where options is an object,
Expand DownExpand Up@@ -135,7 +135,6 @@ function normalizeArgs(args){
else
return [options, cb];
}
exports._normalizeArgs = normalizeArgs;


// called when creating new Socket, or when re-using a closed Socket
Expand DownExpand Up@@ -333,9 +332,6 @@ function writeAfterFIN(chunk, encoding, cb){
}
}

exports.Socket = Socket;
exports.Stream = Socket; // Legacy naming.

Socket.prototype.read = function(n){
if (n === 0)
return stream.Readable.prototype.read.call(this, n);
Expand DownExpand Up@@ -850,7 +846,8 @@ function afterWrite(status, handle, req, err){
}


function connect(self, address, port, addressType, localAddress, localPort){
function internalConnect(
self, address, port, addressType, localAddress, localPort){
// TODO return promise from Socket.prototype.connect which
// wraps _connectReq.

Expand DownExpand Up@@ -964,7 +961,7 @@ Socket.prototype.connect = function(){
this.writable = true;

if (pipe){
connect(this, options.path);
internalConnect(this, options.path);
} else{
lookupAndConnect(this, options);
}
Expand All@@ -979,7 +976,7 @@ function lookupAndConnect(self, options){
var localAddress = options.localAddress;
var localPort = options.localPort;

if (localAddress && !exports.isIP(localAddress))
if (localAddress && !cares.isIP(localAddress))
Copy link
Member

@TimothyGuTimothyGuMar 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making isIP available as a variable would help simplify here and in module.exports.

throw new TypeError('"localAddress" option must be a valid IP: ' +
localAddress);

Expand All@@ -996,11 +993,11 @@ function lookupAndConnect(self, options){
port |= 0;

// If host is an IP, skip performing a lookup
var addressType = exports.isIP(host);
var addressType = cares.isIP(host);
if (addressType){
process.nextTick(function(){
if (self.connecting)
connect(self, host, port, addressType, localAddress, localPort);
internalConnect(self, host, port, addressType, localAddress, localPort);
});
return;
}
Expand DownExpand Up@@ -1040,12 +1037,12 @@ function lookupAndConnect(self, options){
process.nextTick(connectErrorNT, self, err);
} else{
self._unrefTimer();
connect(self,
ip,
port,
addressType,
localAddress,
localPort);
internalConnect(self,
ip,
port,
addressType,
localAddress,
localPort);
}
});
}
Expand DownExpand Up@@ -1177,7 +1174,6 @@ function Server(options, connectionListener){
this.pauseOnConnect = !!options.pauseOnConnect;
}
util.inherits(Server, EventEmitter);
exports.Server = Server;


function toNumber(x){return (x = Number(x)) >= 0 ? x : false}
Expand DownExpand Up@@ -1244,7 +1240,6 @@ function createServerHandle(address, port, addressType, fd){

return handle;
}
exports._createServerHandle = createServerHandle;


Server.prototype._listen2 = function(address, port, addressType, backlog, fd){
Expand DownExpand Up@@ -1618,20 +1613,12 @@ Server.prototype.unref = function(){
return this;
};


exports.isIP = cares.isIP;


exports.isIPv4 = cares.isIPv4;


exports.isIPv6 = cares.isIPv6;

var _setSimultaneousAccepts;

if (process.platform === 'win32'){
var simultaneousAccepts;

exports._setSimultaneousAccepts = function(handle){
_setSimultaneousAccepts = function(handle){
if (handle === undefined){
return;
}
Expand All@@ -1647,5 +1634,20 @@ if (process.platform === 'win32'){
}
};
} else{
exports._setSimultaneousAccepts = function(handle){};
_setSimultaneousAccepts = function(handle){};
}

module.exports ={
_createServerHandle: createServerHandle,
_normalizeArgs: normalizeArgs,
_setSimultaneousAccepts,
connect,
createConnection: connect,
createServer,
isIP: cares.isIP,
isIPv4: cares.isIPv4,
isIPv6: cares.isIPv6,
Server,
Socket,
Stream: Socket, // Legacy naming
};