util: display constructor name when inspecting objects

[monsanto]

This feature is inspired by browser devtools. This PR modifies util.inspect(obj) to additionally show the name of the function that constructed the object. This often reveals useful information about the object's prototype. In other words, instead of

> new Cls
{}

we have

> new Cls
Cls {}

This also works with exotic objects:

> class ArrayCls extends Array {}
> new ArrayCls(1, 2, 3)
ArrayCls [ 1, 2, 3 ]

The names of "trivial" constructors like Object and Array are not shown, unless there is a mismatch between the object representation and the prototype:

> Object.create([])
Array {}

The constructor displayed is the first function-valued 'constructor' property found by traversing the object and its prototypes. Anonymous functions and non-value descriptors are ignored. Note that cyclic prototypes are illegal so the search always terminates (unless one overrides getPrototypeOf with a proxy, but then I would argue you get what you deserve).

---

Display of boxed primitives is unchanged. One could imagine the following:

> class NumberCls extends Number {}
> new NumberCls(0)
[NumberCls: 0]

but right now, this will display [Number: 0]. This is largely because I am unhappy that the display of boxed primitives is inconsistent with how other exotic objects are displayed (Maps, Sets, Promises). But since boxed primitives are so rarely used this argument is ripe for bikeshedding and I would rather not have that stall the bulk of this feature.

[targos]

I am +1 for this feature as I was looking into it myself.

I can think of one case that will pick the wrong constructor name though:

function A() {}
var a = new A();
a.constructor = function B() {};
util.inspect(a); // B {}

This case could be handled correctly by using the v8::Object::GetConstructorName method on the C++ side.
I tried it and unfortunately there seems to be a problem with extended classes. The method always returns the name of the base class. Do you think I should open an issue on v8's tracker for that?

[benjamingr]

Super useful, definitely in favor. Note that Chrome had a regression about the way he does this - summoning @paulirish and @Zirak who are familiar with more details about this.

[monsanto]

@targos Yeah, you can fake the constructor. I did look at the capabilities of Debug.MakeMirror (since we use it for inspecting promises) and didn't see anything that would help me find Guaranteed Correct™ constructors, but it doesn't surprise me that there is a C++ API for it.

I'm not sure if allowing fakes is a feature or a bug. I tend to just assume people will do the right thing since there is no avoiding weird object model corner cases in JavaScript. For example, one might assume that a.constructor === A implies a instanceof A but this is not so:

> a = new A
> a instanceof A
true
> A.prototype = {}
> a instanceof A
false
> a.constructor === A
true

However I will note that one could work around your example by starting the constructor search on the prototype, although this wouldn't help if you did A.prototype.constructor = B.

[monsanto]

@targos as for the V8 bug, I couldn't say. I'll note that if GetConstructorName is anything like the intrinsic %_ClassOf, ignoring subclasses is intentional.

[Zirak]

@benjamingr The bug was in Map::GetConstructor, but since he's not touching internals here he's good in this case.

This would be a really nice feature to have; one thing to consider is how it'll scale with nested objects, for instance:

function Foo () {
    this.b = new Bar();
    this.d = new Bar();
}
function Bar () {
    this.c = 4;
}
require('util').inspect(new Foo());

Right now the output is { b: { c: 4 }, d: { c: 4 } }, what would you like it to be?

[monsanto]

@nodejs/collaborators Hey, it's been a week and I have not gotten any reviews from collaborators--mind helping me out here? Hard to move forward otherwise.

[mscdex]

I'm not familiar enough with the behavior of this in the browser or anything to know how it compares, but in general I am +1 on adding this.

[silverwind]

It'd be helpful if you could demonstrate how each browser presents constructors (Chrome, Firefox, Edge, maybe Safari).

[trevnorris]

Overall I'm +1. There are some edge cases that should be addressed. For example how would anonymous functions be displayed?

[Fishrock123]

👍 I'll try to remember to review tomorrow.

[Fishrock123]

If there is no constructor this just returns undefined I guess..?

[monsanto]

Yes.

I modified this so we explicitly return null instead of relying on the implicit undefined.

[Fishrock123]

A check for anon functions would also be nice, Maybe also String objects? (new String('cats'))

Edit: also, what happens if you feed it a generator?

[benjamingr]

Have you actually ever seen a String object or Number object in the wild
outside of people talking about the quirky typesystem ;)?

On Wed, Jun 17, 2015 at 7:17 PM, Jeremiah Senkpiel <notifications@github.com

wrote:

A check for anon functions would also be nice, Maybe also String objects? (new
String('cats'))

—
Reply to this email directly or view it on GitHub
#1935 (comment).

[Fishrock123]

Have you actually ever seen a String object or Number object in the wild
outside of people talking about the quirky typesystem ;)?

Hmmm, I think so, sadly.

It should handle it anyways.

[monsanto]

Sorry for the delay, all.

@silverwind Chrome displays object constructors similar to what I have, except that they do not elide Object, so {} will display as Object {}. They elide constructors completely for arrays, even subclassed ones. Firefox ignores constructors, any object always displays as Object {...} and any array always displays as Array [...]. I don't have access to Edge, but IE11 ignores constructors. I don't have access to Safari at all.

Also see responses to others.

@trevnorris Right now anonymous functions are simply ignored, and we continue to traverse the prototype until we find a constructor with a name. This is consistent with Chrome. Do you think this is suboptimal? If so we could display like <anonymous> { }

@Fishrock123

A check for anon functions would also be nice

See above

Maybe also String objects? (new String('cats'))

Right now all boxed primitives have a special display: [String: 'cats'], [Number: 0], etc. As stated in the original post, one could imagine adding subclassing support ([StringSubclass: 'cats'], ...) but I didn't make any changes because I think we should first fix their inconsistent display. I think new String('cats') should display as String { 'cats' }, new Number(0) should display as Number { 0 }, etc. This is how Chrome displays them, except that they expose the primitive as an "internal property": Number {[[PrimitiveValue]]: 0}.

If there is universal agreement that we should change the display I can add it + subclassing support in this PR but I figured it would be contentious.

what happens if you feed it a generator?

> function *x() { }
> x()
GeneratorFunctionPrototype {}

In Chrome, this looks like:

> x()
x {[[GeneratorStatus]]: "suspended", [[GeneratorReceiver]]: Window}

I don't think it's possible to get the function name through vanilla JS but with the debug API we probably could.

[Fishrock123]

SGTM to be improved in the future. Let's see what the others think.

[trevnorris]

@monsanto Think I was expecting to see the same as it would be as if an error had been thrown. e.g.

> (function() { (function() { throw Error('a'); }()); }());
VM544:2 Uncaught Error: a
    at Error (native)
    at <anonymous>:2:35
    at <anonymous>:2:48
    at <anonymous>:2:54

Though Chrome does have a few interesting things:

> var a = [function() { }, function() { throw Error('b'); }]
undefined
> a
[anonymous(), a()]

So why the first one would be anonymous but the last be given a variable name I'm not sure.

But yeah. Guess I'd expect that if a name for the function can't be immediately found then it would just say anonymous.

[silverwind]

@trevnorris:

That Chrome example looks broken/confusing to me. I much prefer the output it gives when you put it in a single statement:

> [function() { }, function() { throw Error('b'); }]
< [function(), function()]

@monsanto:

Safari seems to put anything between {} into expandable lists. Not really comparable.
Chrome does seem to only hide Array, but not Object. I like your approach to hiding both better actually.
Firefox does put Object before anything, as you mentioned. Pretty uninformative.
Edge I can't test here.

+1 from me on this PR, need to check if more edge cases exist before I can sign this off. I think we should leave other related improvements for future PRs.

[trevnorris]

@silverwind The example showing how the anonymous function in the array is magically assigned the name of the variable is the strange behavior I was trying to point out just for contrast. I definitely don't agree it should work that way. ;-)

[silverwind]

Played with it a bit, seems to work well. The representation of constructors in literals is a bit confusing without syntax highlighting, but overall I think the benefits outweight the drawbacks:

Foo { b: Bar { c: 4 }, d: Bar { c: 4 } }

On the REPL, I could see some coloring being added to constructors (and more importantly, properties), but that's material for another PR.

LGTM

[silverwind]

@monsanto shall we land this?

[monsanto]

@trevnorris Do you consider wanting "anonymous" as the constructor name as blocking? I would personally prefer Chrome's behavior here as "anonymous" doesn't convey any information about the prototype. If we find a prototype with a name further in the chain at least gives partial information.

@silverwind if @trevnorris OKs this then yeah I think it is ready to land.

[trevnorris]

Not a blocker for me.

[silverwind]

@monsanto let's land it after a rebase (presumably just because of .eslintrc).

[monsanto]

https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/228/

(EDIT: looks like those errors were spurious. https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/232/)

[monsanto]

landed in 7d14dd9