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

lib: refactor internal/util #11404

Closed
wants to merge 3 commits into from
Closed
Changes from 2 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -5,19 +5,24 @@ const signals = process.binding('constants').os.signals;

const kArrowMessagePrivateSymbolIndex = binding['arrow_message_private_symbol'];
const kDecoratedPrivateSymbolIndex = binding['decorated_private_symbol'];
const noCrypto = !process.versions.openssl;

// The `buffer` module uses this. Defining it here instead of in the public
// `util` module makes it accessible without having to `require('util')` there.
exports.customInspectSymbol = Symbol('util.inspect.custom');
function isError(e) {
return objectToString(e) === '[object Error]' || e instanceof Error;
}

This comment has been minimized.

Copy link
@sam-github

sam-github Apr 20, 2017

Member

sometimes there is two lines beween definitions, sometimes one, should probably be consisten

// Mark that a method should not be used.
// Returns a modified function which warns once by default.
// If --no-deprecation is set, then it is a no-op.
exports.deprecate = function deprecate(fn, msg, code) {
function objectToString(o) {
return Object.prototype.toString.call(o);
}

// Mark that a method should not be used.

This comment has been minimized.

Copy link
@mscdex

mscdex Apr 20, 2017

Contributor

These comments shouldn't be indented.

This comment has been minimized.

Copy link
@jasnell

jasnell Apr 20, 2017

Author Member

whoops! good catch :-)

// Returns a modified function which warns once by default.
// If --no-deprecation is set, then it is a no-op.
function deprecate(fn, msg, code) {
// Allow for deprecating things in the process of starting up.
if (global.process === undefined) {
return function() {
return exports.deprecate(fn, msg, code).apply(this, arguments);
return function(...args) {
return deprecate(fn, msg).apply(this, args);
};
}

@@ -29,7 +34,7 @@ exports.deprecate = function deprecate(fn, msg, code) {
throw new TypeError('`code` argument must be a string');

var warned = false;
function deprecated() {
function deprecated(...args) {
if (!warned) {
warned = true;
if (code !== undefined) {
@@ -39,9 +44,9 @@ exports.deprecate = function deprecate(fn, msg, code) {
}
}
if (new.target) {
return Reflect.construct(fn, arguments, new.target);
return Reflect.construct(fn, args, new.target);
}
return fn.apply(this, arguments);
return fn.apply(this, args);
}

// The wrapper will keep the same prototype as fn to maintain prototype chain
@@ -54,10 +59,10 @@ exports.deprecate = function deprecate(fn, msg, code) {
}

return deprecated;
};
}

exports.decorateErrorStack = function decorateErrorStack(err) {
if (!(exports.isError(err) && err.stack) ||
function decorateErrorStack(err) {
if (!(isError(err) && err.stack) ||
binding.getHiddenValue(err, kDecoratedPrivateSymbolIndex) === true)
return;

@@ -67,30 +72,19 @@ exports.decorateErrorStack = function decorateErrorStack(err) {
err.stack = arrow + err.stack;
binding.setHiddenValue(err, kDecoratedPrivateSymbolIndex, true);
}
};

exports.isError = function isError(e) {
return exports.objectToString(e) === '[object Error]' || e instanceof Error;
};

exports.objectToString = function objectToString(o) {
return Object.prototype.toString.call(o);
};
}

const noCrypto = !process.versions.openssl;
exports.assertCrypto = function() {
function assertCrypto() {
if (noCrypto)
throw new Error('Node.js is not compiled with openssl crypto support');
};

exports.kIsEncodingSymbol = Symbol('node.isEncoding');
}

// The loop should only run at most twice, retrying with lowercased enc
// if there is no match in the first pass.
// We use a loop instead of branching to retry with a helper
// function in order to avoid the performance hit.
// Return undefined if there is no match.
exports.normalizeEncoding = function normalizeEncoding(enc) {
function normalizeEncoding(enc) {
if (!enc) return 'utf8';
var retried;
while (true) {
@@ -116,11 +110,9 @@ exports.normalizeEncoding = function normalizeEncoding(enc) {
retried = true;
}
}
};
}

// Filters duplicate strings. Used to support functions in crypto and tls
// modules. Implemented specifically to maintain existing behaviors in each.
exports.filterDuplicateStrings = function filterDuplicateStrings(items, low) {
function filterDuplicateStrings(items, low) {
const map = new Map();
for (var i = 0; i < items.length; i++) {
const item = items[i];
@@ -132,24 +124,24 @@ exports.filterDuplicateStrings = function filterDuplicateStrings(items, low) {
}
}
return Array.from(map.values()).sort();
};
}

exports.cachedResult = function cachedResult(fn) {
function cachedResult(fn) {
var result;
return () => {
if (result === undefined)
result = fn();
return result.slice();
};
};
}

// Useful for Wrapping an ES6 Class with a constructor Function that
// does not require the new keyword. For instance:
// class A { constructor(x) {this.x = x;}}
// const B = createClassWrapper(A);
// B() instanceof A // true
// B() instanceof B // true
exports.createClassWrapper = function createClassWrapper(type) {
function createClassWrapper(type) {
const fn = function(...args) {
return Reflect.construct(type, args, new.target || type);
};
@@ -161,7 +153,7 @@ exports.createClassWrapper = function createClassWrapper(type) {
Object.setPrototypeOf(fn, type);
fn.prototype = type.prototype;
return fn;
};
}

let signalsToNamesMapping;
function getSignalsToNamesMapping() {
@@ -176,7 +168,7 @@ function getSignalsToNamesMapping() {
return signalsToNamesMapping;
}

exports.convertToValidSignal = function convertToValidSignal(signal) {
function convertToValidSignal(signal) {
if (typeof signal === 'number' && getSignalsToNamesMapping()[signal])
return signal;

@@ -186,4 +178,25 @@ exports.convertToValidSignal = function convertToValidSignal(signal) {
}

throw new Error('Unknown signal: ' + signal);
}

module.exports = {

This comment has been minimized.

Copy link
@sam-github

sam-github Apr 20, 2017

Member

assigning to exports = as well is more future proof, it allows util functions to refer to each other more easily.

Of the 3 common export styles, at-top, at-definition (what util used to do), and at-bottom,at-bottom remains my least favorite. It lacks the visibility and readability of export at top, and lacks the cohesion of export at time of definition. But if at-bottom is how node.js goes, I'll follow along.

This comment has been minimized.

Copy link
@jasnell

jasnell Apr 20, 2017

Author Member

Avoiding the need to refer to exports.{whatever} is important and is why the various exported functions are all named top level functions (they can refer to each other without the exports. bit). That said, it's harmless to add.

This comment has been minimized.

Copy link
@mscdex

mscdex Apr 20, 2017

Contributor

One problem with having it at the top is that you can run into variable definition issues, where you're exporting something that gets defined/set later on in the file. Sometimes you might be able to just pull the definition to the top, but other times the assignment may not be a simple one-liner. So putting it at the bottom means you never have to worry about such issues and doing it the same everywhere is good for consistency.

assertCrypto,
cachedResult,
convertToValidSignal,
createClassWrapper,
decorateErrorStack,
deprecate,
filterDuplicateStrings,
isError,
normalizeEncoding,
objectToString,

// Symbol used to provide a custom inspect function for an object as an
// alternative to using 'inspect'
customInspectSymbol: Symbol('util.inspect.custom'),

// Used by the buffer module to capture an internal reference to the
// default isEncoding implementation, just in case userland overrides it.
kIsEncodingSymbol: Symbol('node.isEncoding')
};
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.