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

Util(.is*) Deprecations #1301

Closed
wants to merge 3 commits into from
Closed

Util(.is*) Deprecations #1301

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
281c44e
util: cleanup in-util deprecation
Fishrock123 Mar 31, 2015
d2fe24e
util: deprecate most of utils.is*
Fishrock123 Mar 31, 2015
50ad0cc
util: don't bother checking types before objectToString()
Fishrock123 Apr 8, 2015
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
69 changes: 41 additions & 28 deletions lib/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ exports.format = function(f) {
// 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(fn, msg) {
function deprecate(fn, msg) {
// Allow for deprecating things in the process of starting up.
if (global.process === undefined) {
return function() {
return exports.deprecate(fn, msg).apply(this, arguments);
return deprecate(fn, msg).apply(this, arguments);
};
}

Expand All @@ -71,7 +71,8 @@ exports.deprecate = function(fn, msg) {
}

return deprecated;
};
}
exports.deprecate = deprecate;


var debugs = {};
Expand Down Expand Up @@ -504,66 +505,77 @@ const isArray = exports.isArray = Array.isArray;
function isBoolean(arg) {
return typeof arg === 'boolean';
}
exports.isBoolean = isBoolean;
exports.isBoolean = deprecate(isBoolean,
'util.isBoolean is deprecated, please use a user-land alternative.');

function isNull(arg) {
return arg === null;
}
exports.isNull = isNull;
exports.isNull = deprecate(isNull,
'util.isNull is deprecated, please use a user-land alternative.');

function isNullOrUndefined(arg) {
return arg === null || arg === undefined;
}
exports.isNullOrUndefined = isNullOrUndefined;
exports.isNullOrUndefined = deprecate(isNullOrUndefined,
'util.isNullOrUndefined is deprecated, please use a user-land alternative.');

function isNumber(arg) {
return typeof arg === 'number';
}
exports.isNumber = isNumber;
exports.isNumber = deprecate(isNumber,
'util.isNumber is deprecated, please use a user-land alternative.');

function isString(arg) {
return typeof arg === 'string';
}
exports.isString = isString;
exports.isString = deprecate(isString,
'util.isSrting is deprecated, please use a user-land alternative.');

function isSymbol(arg) {
return typeof arg === 'symbol';
}
exports.isSymbol = isSymbol;
exports.isSymbol = deprecate(isSymbol,
'util.isSymbol is deprecated, please use a user-land alternative.');

function isUndefined(arg) {
return arg === undefined;
}
exports.isUndefined = isUndefined;
exports.isUndefined = deprecate(isUndefined,
'util.isUndefined is deprecated, please use a user-land alternative.');

// note: the isRegExp function is still used here in util
function isRegExp(re) {
return re !== null && typeof re === 'object' &&
objectToString(re) === '[object RegExp]';
return objectToString(re) === '[object RegExp]';
}
exports.isRegExp = isRegExp;
exports.isRegExp = deprecate(isRegExp,
'util.isRegExp is deprecated, please use a user-land alternative.');

function isObject(arg) {
return arg !== null && typeof arg === 'object';
}
exports.isObject = isObject;
exports.isObject = deprecate(isObject,
'util.isObject is deprecated, please use a user-land alternative.');

// still used in assert and fs
function isDate(d) {
return d !== null && typeof d === 'object' &&
objectToString(d) === '[object Date]';
return objectToString(d) === '[object Date]';
}
exports.isDate = isDate;

function isError(e) {
return e !== null && typeof e === 'object' &&
(objectToString(e) === '[object Error]' || e instanceof Error);
return objectToString(e) === '[object Error]' || e instanceof Error;
}
exports.isError = isError;
exports.isError = deprecate(isError,
'util.isError is deprecated, please use a user-land alternative.');

function isFunction(arg) {
return typeof arg === 'function';
}
exports.isFunction = isFunction;
exports.isFunction = deprecate(isFunction,
'util.isFunction is deprecated, please use a user-land alternative.');

// still used in assert, domain, and smalloc
function isPrimitive(arg) {
return arg === null ||
typeof arg !== 'object' && typeof arg !== 'function';
Expand All @@ -573,7 +585,8 @@ exports.isPrimitive = isPrimitive;
function isBuffer(arg) {
return arg instanceof Buffer;
}
exports.isBuffer = isBuffer;
exports.isBuffer = deprecate(isBuffer,
'util.isBuffer is deprecated, please use a user-land alternative.');
Copy link
Contributor

Choose a reason for hiding this comment

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

This should recommend using Buffer.isBuffer, not a user-land alternative.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooops. Honestly, I don't even know why we use Buffer.isBuffer either... it's just instannceof Buffer...

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why Buffer.isBuffer was added initially. That'd be interesting to find out.

That said, Buffer.isBuffer has been incredibly useful to me in writing buffer for browserify. The Buffer constructor actually returns a Uint8Array for performance reasons, but with all the buffer methods attached as properties. Works great, except for one edge case – instanceof Buffer doesn't work anymore. So, I've been recommending that people use Buffer.isBuffer and that's worked well. :-)

Eventually, I can just return a proper Buffer that's a subclass of Uint8Array (arrays are finally subclassable!) and instanceof Buffer will start working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @trevnorris?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why Buffer.isBuffer was added initially. That'd be interesting to find out.

Once upon a time there existed the SlowBuffer class. It didn't inherit from Buffer so instances were buffers but not instanceof Buffer.

SlowBuffer still exists but it's more or less obsolete. It inherits from Buffer now so instanceof checks work.

As to why it didn't before, I don't exactly remember but it was probably accidental. SlowBuffer was an implementation detail - it was the backing store for normal buffers - and not something users normally interacted with directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @bnoordhuis. That was before my time.

Copy link

Choose a reason for hiding this comment

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

OK, should I use instanceof Buffer or better Buffer.isBuffer(...) instead of util.isBuffer in Node 0.12.7 and above?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can also use Buffer.isBuffer which is not deprecated.
On Thu, Nov 12, 2015 at 3:55 AM hellboy81 notifications@github.com wrote:

In lib/util.js
#1301 (comment):

@@ -573,7 +585,8 @@ exports.isPrimitive = isPrimitive;
function isBuffer(arg) {
return arg instanceof Buffer;
}
-exports.isBuffer = isBuffer;
+exports.isBuffer = deprecate(isBuffer,

  • 'util.isBuffer is deprecated, please use a user-land alternative.');

OK, should I use instanceof Buffer instead of util.isBuffer in Node 0.12.7
and above?

—
Reply to this email directly or view it on GitHub
https://github.com/nodejs/node/pull/1301/files#r44648758.

Copy link

Choose a reason for hiding this comment

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

And isBuffer is better than instanceof Buffer ?

Copy link
Contributor

Choose a reason for hiding this comment

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

They are exactly the same.
On Thu, Nov 12, 2015 at 4:38 AM hellboy81 notifications@github.com wrote:

In lib/util.js
#1301 (comment):

@@ -573,7 +585,8 @@ exports.isPrimitive = isPrimitive;
function isBuffer(arg) {
return arg instanceof Buffer;
}
-exports.isBuffer = isBuffer;
+exports.isBuffer = deprecate(isBuffer,

  • 'util.isBuffer is deprecated, please use a user-land alternative.');

And isBuffer is better than instanceof Buffer ?

—
Reply to this email directly or view it on GitHub
https://github.com/nodejs/node/pull/1301/files#r44651882.


function objectToString(o) {
return Object.prototype.toString.call(o);
Expand Down Expand Up @@ -663,45 +676,45 @@ function hasOwnProperty(obj, prop) {

// Deprecated old stuff.

exports.p = exports.deprecate(function() {
exports.p = deprecate(function() {
for (var i = 0, len = arguments.length; i < len; ++i) {
console.error(exports.inspect(arguments[i]));
}
}, 'util.p: Use console.error() instead');


exports.exec = exports.deprecate(function() {
exports.exec = deprecate(function() {
return require('child_process').exec.apply(this, arguments);
}, 'util.exec is now called `child_process.exec`.');


exports.print = exports.deprecate(function() {
exports.print = deprecate(function() {
for (var i = 0, len = arguments.length; i < len; ++i) {
process.stdout.write(String(arguments[i]));
}
}, 'util.print: Use console.log instead');


exports.puts = exports.deprecate(function() {
exports.puts = deprecate(function() {
for (var i = 0, len = arguments.length; i < len; ++i) {
process.stdout.write(arguments[i] + '\n');
}
}, 'util.puts: Use console.log instead');


exports.debug = exports.deprecate(function(x) {
exports.debug = deprecate(function(x) {
process.stderr.write('DEBUG: ' + x + '\n');
}, 'util.debug: Use console.error instead');


exports.error = exports.deprecate(function(x) {
exports.error = deprecate(function(x) {
for (var i = 0, len = arguments.length; i < len; ++i) {
process.stderr.write(arguments[i] + '\n');
}
}, 'util.error: Use console.error instead');


exports.pump = exports.deprecate(function(readStream, writeStream, callback) {
exports.pump = deprecate(function(readStream, writeStream, callback) {
var callbackCalled = false;

function call(a, b, c) {
Expand Down