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.

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

Jump to bottom

net: refactor net module to module.exports #11698

Closed
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 number Diff line number Diff 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

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
Contributor Author

@claudiorodriguez claudiorodriguez Mar 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

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 Down Expand 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 Down Expand 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 Down Expand 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 Down Expand 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

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 Down Expand 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 Down Expand 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 Down Expand 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 Down Expand 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
};