Skip to content

Util(.is*) Deprecations #1301

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
Fishrock123 wants to merge 3 commits into from
Closed

Util(.is*) Deprecations #1301

Changes from 1 commit
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
Prev Previous commit
Next Next commit
util: deprecate most of utils.is*
  • Loading branch information
@Fishrock123
Fishrock123 committed Apr 8, 2015
commit d2fe24ecd91783bbf6521eb9fe7159f89f2e9bf0
39 changes: 27 additions & 12 deletions lib/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,49 +505,60 @@ 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]';
Copy link
Contributor

@thefourtheye thefourtheye Mar 31, 2015

Choose a reason for hiding this comment

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

Wouldn't the objectToString be sufficient here? Why do we need the other two checks?

}
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]';
Expand All @@ -558,13 +569,16 @@ function isError(e) {
return e !== null && typeof e === 'object' &&
(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 @@ -574,7 +588,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

@feross feross Apr 12, 2015

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

@Fishrock123 Fishrock123 Apr 12, 2015

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

@feross feross Apr 13, 2015

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

@Fishrock123 Fishrock123 Apr 13, 2015

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

@bnoordhuis bnoordhuis Apr 13, 2015

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

@trevnorris trevnorris Apr 15, 2015

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

@hellboy81 hellboy81 Nov 12, 2015

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

@feross feross Nov 12, 2015

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 [email protected] 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

@hellboy81 hellboy81 Nov 12, 2015

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

@feross feross Nov 12, 2015

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 [email protected] 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