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

Printing the constructor name for non-plain objects #15

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Printing the constructor name for non-plain objects #15

Show file tree
Hide file tree
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 37 additions & 2 deletions index.js
View file
Open in desktop
Expand Up @@ -15,6 +15,7 @@ var objectToString = Object.prototype.toString;
var functionToString = Function.prototype.toString;
var match = String.prototype.match;
var bigIntValueOf = typeof BigInt === 'function' ? BigInt.prototype.valueOf : null;
var getPrototype = Object.getPrototypeOf || function(o) { return o.__proto__; };
Copy link
Member

Choose a reason for hiding this comment

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

What about the older engines where __proto__ isn't even available?

Copy link
Author

Choose a reason for hiding this comment

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

In that case, __proto__ will be undefined, as will be the result of getPrototype. Consequently, getTypeString will return null (because the result of getPrototype is falsey). And the code will behave as it always has, not printing a type prefix.

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't a { __proto__: 3 } return 3 in an engine where it's not available?

Copy link

Choose a reason for hiding this comment

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

Wouldn't a { __proto__: 3 } return 3 in an engine where it's not available?

That will happen even in engines that support __proto__.

Copy link
Member

Choose a reason for hiding this comment

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

@ExE-Boss that spec text only treats it as a normal data property when it's not a string, or computed.

({ __proto__: 3 }).__proto__ === Object.prototype returns true in a modern engine, but in, say, IE 8, it returns false.

Copy link
Member

Choose a reason for hiding this comment

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

Specifically, in IE 8, ({ __proto__: 3 }).__proto__ returns 3.


var inspectCustom = require('./util.inspect').custom;
var inspectSymbol = inspectCustom && isSymbol(inspectCustom) ? inspectCustom : null;
Expand Down Expand Up @@ -145,9 +146,12 @@ module.exports = function inspect_(obj, options, depth, seen) {
return markBoxed(inspect(String(obj)));
}
if (!isDate(obj) && !isRegExp(obj)) {
var typeString = getTypeString(obj);
var prefix = typeString ? typeString + ' ' : '';
var xs = arrObjKeys(obj, inspect);
if (xs.length === 0) { return '{}'; }
return '{ ' + xs.join(', ') + ' }';
return xs.length === 0
? prefix + '{}'
: prefix + '{ ' + xs.join(', ') + ' }';
}
return String(obj);
};
Expand Down Expand Up @@ -319,3 +323,34 @@ function arrObjKeys(obj, inspect) {
}
return xs;
}

// Returns the object's constructor name or null if it is a plain object
// or doesn't have a prototype.
function getTypeString(o) {
if (Object.prototype.toString.call(o) !== '[object Object]') return null;
var prototype = getPrototype(o);
if (!prototype) return null;

var constructorName = o.constructor ? o.constructor.name : null;
var isPlainObject = constructorName === 'Object' && looksLikeObjectPrototype(prototype);
if (isPlainObject) {
return null;
}

return constructorName;
}

// Indicates whether the specified object appears to be Object.prototype,
// regardless of the object's realm.
function looksLikeObjectPrototype(o) {
if (o === Object.prototype) return true;

// Cross-realm objects use a different Object, so we have to use a heuristic.
return !getPrototype(o)
&& o.hasOwnProperty('hasOwnProperty')
&& o.hasOwnProperty('isPrototypeOf')
&& o.hasOwnProperty('propertyIsEnumerable')
&& o.hasOwnProperty('toLocaleString')
Copy link
Member

Choose a reason for hiding this comment

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

should these check that they're functions, and ideally, that they behave as expected?

Copy link
Author

Choose a reason for hiding this comment

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

I don't think there's anything to be gained by that. Consider that you have an object with own properties hasOwnProperty, isPrototypeOf, etc. Either this is Object.prototype, or it's an object that's trying really hard to look like it. In the latter case, chances are that all those will be functions and that they'll behave correctly.

What's the worst that can happen? The worst case is that you have an object that's trying hard to look like a plain object, and that your library prints it just like a plain object. I don't think that's bad.

After all, this library isn't generating structured information. It's generating a debug string that is supposed to help a developer understand an object graph. And I don't think that anybody will complain if something that's virtually indistinguishable from a plain object is visualized as a plain object.

Copy link
Member

Choose a reason for hiding this comment

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

That's a fair point. However, to be indistinguishable, this library would have to be unable to distinguish it.

One extreme length we could go to is ensuring not just that they're all functions, but also that Function.prototype.toString.call indicates it's native code, which would work cross-realm.

&& o.hasOwnProperty('toString')
&& o.hasOwnProperty('valueOf');
}
21 changes: 21 additions & 0 deletions test/typed.js
View file
Open in desktop
@@ -0,0 +1,21 @@
var inspect = require('../');
var test = require('tape');

test('prototype is Object.prototype', function (t) {
t.plan(1);
var obj = {};
t.equal(inspect(obj), '{}');
});

test('prototype is null', function (t) {
t.plan(1);
var obj = Object.create(null);
t.equal(inspect(obj), '{}');
});

test('prototype from new', function (t) {
t.plan(1);
function Foo() {}
var obj = new Foo();
t.equal(inspect(obj), 'Foo {}');
});