-
Notifications
You must be signed in to change notification settings - Fork 30.6k
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
lib: refactor internal/util #11404
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
} | ||
|
||
// 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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These comments shouldn't be indented. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. assigning to 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoiding the need to refer to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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') | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sometimes there is two lines beween definitions, sometimes one, should probably be consisten