1526688 - Show instance constructor name in console instead of "Object"

Status: NEW

(DevTools :: Console, defect, P2)

Description

[Jelle De Loecker]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:65.0) Gecko/20100101 Firefox/65.0

Steps to reproduce:

I logged an instance of a custom class to the console, for example:

function MyClass() {};
console.log(new MyClass());

Actual results:

Firefox prints the object I can inspect, but it looks like Object { }

Expected results:

I expect to see the constructor name instead of Object, so: MyClass { }
Chrome does it like this, and it makes the console output much cleaner and easier to debug.

The built-in classes (like Map, Date, Promise, DocumentFragment, ...) DO show the constructor name

I noticed some of them contained the Symbol.toStringTag property in their prototype, but even adding that did nothing.

Comment 1

[Gyula Palko[:gpalko]]

Setting the component, in order to involve the development team.

Comment 2

[:Harald Kirschner :digitarald]

Impacts React, Angular, and other framework debugging and aligns with optimizing for modern build chains (which depend on es6 classes).

Also reported here: https://www.reddit.com/r/javascript/comments/ecyhmr/mozilla_survey_how_can_devtools_support_you/fbhbnng/

Comment 3

[:Harald Kirschner :digitarald]

Brian, is this something we need better server-side/JS engine support for, or is it frontend (or sth else)?

Comment 4

[Brian Hackett [Laid off!]]

(In reply to :Harald Kirschner :digitarald from comment #3)

Brian, is this something we need better server-side/JS engine support for, or is it frontend (or sth else)?

This would need spidermonkey changes in order to fix. When JS code calls 'new MyClass()' we create a vanilla object (a PlainObject in spidermonkey terms) which doesn't have any notion of what function was used to construct it or other information about where it came from. We can look at its prototype chain, which in this case will have been constructed lazily the first time MyClass.prototype was needed --- MyClass will point to the prototype, but not the other way around, however. Using the prototype gets more complicated with the typical pattern here of having something like MyClass.prototype = { ... } or MyClass.prototype = someLibraryFunction(...) and the prototype could be any random object. Determining the constructor function from an instance object would require some pretty significant new complexity and performance/memory overhead.

Comment 5

[:Harald Kirschner :digitarald]

Couldn't ES6's instance.constructor.name get is halfway there?

function MyClass() {};
console.log((new MyClass()).constructor.name);

Comment 6

[Brian Hackett [Laid off!]]

(In reply to :Harald Kirschner :digitarald from comment #5)

Couldn't ES6's instance.constructor.name get is halfway there?

function MyClass() {};
console.log((new MyClass()).constructor.name);

Hmm, that does fix the issue of getting to the constructor from the default function prototype, but if MyClass.prototype is anything other than its default value then we lose the link between the constructor function and the prototype.

Comment 7

[:Harald Kirschner :digitarald]

Hmm, that does fix the issue of getting to the constructor from the default function prototype, but if MyClass.prototype is anything other than its default value then we lose the link between the constructor function and the prototype.

Not sure what would change the prototype, do you have an example of what use case would miss?

Would it make sense to split this into supporting the basic constructor.name for labeling and a follow up bug for advanced JS instrumentation?

Comment 8

[Oriol Brufau [:Oriol]]

I oppose this change. If I want to see the constructor name, I can just use instance.constructor.name.
What I want to see is the internal JSClass, so please don't remove this.
For example, Object.create(Array.prototype) is a plain object instead of an exotic array object, so I do want to see Object and not Array.

Comment 9

[Jelle De Loecker]

(In reply to Oriol Brufau [:Oriol] from comment #8)

I oppose this change. If I want to see the constructor name, I can just use instance.constructor.name.
What I want to see is the internal JSClass, so please don't remove this.
For example, Object.create(Array.prototype) is a plain object instead of an exotic array object, so I do want to see Object and not Array.

The idea is that you can easily see what the constructor is in multiple places, not just by doing a manual console.log on a single variable. It should also be visible at a glance when inspecting an object (or many, multiple objects)

Also: Object.create(Array.prototype) does in fact make an Array instance, albeit a broken one with weird, unexpected behaviours like square-bracket style property getting, JSON stringifying and such. As you said yourself: it's quite the exotic odd one out :)

Yes, the way it currently is programmed in Firefox means you can easily see the difference between a "real" array and a "fake" array, but this one very special, very weird use-case should not prevent this from being implemented as other browsers have done.

Comment 10

[Oriol Brufau [:Oriol]]

(In reply to Jelle De Loecker from comment #9)

Also: Object.create(Array.prototype) does in fact make an Array instance, albeit a broken one with weird, unexpected behaviours like square-bracket style property getting, JSON stringifying and such. As you said yourself: it's quite the exotic odd one out :)

Don't get tricked by the instanceof operator, it's not really an instance since it hasn't been created by the Array constructor.
It's just a plain ordinary object. There is no unexpected behavior, it's not an exotic object. Real arrays are exotic objects (spec).
Showing Array in devtools will just confuse people since they will assume it behaves as a real array exotic object but it doesn't.

but this one very special, very weird use-case should not prevent this from being implemented as other browsers have done.

It's precisely in weird cases when the devtools should be especially useful. In trivial cases they are less needed.

Also, I don't think the proposal is well defined. What if the [[Prototype]] changes? What if the [[Prototype]] doesn't have a constructor property? What if its value is not a constructor? What if its value changes? What if it's a getter (the devtools are not supposed to call getters with side-effects)?

Comment 11

[:Harald Kirschner :digitarald]

It's precisely in weird cases when the devtools should be especially useful. In trivial cases they are less needed.

The general inspection should prioritize that logging most common cases just works (common patterns in frameworks, best practices, etc). We should start documenting edge cases so they can be treated with care. I am not sure if these should block any of the work here though.

Comment 12

[:Harald Kirschner :digitarald]

Attachment: Logging Angular in Firefox vs Chrome

Angular classes run into the same issue, reported via https://discourse.mozilla.org/t/firefox-web-console-property-getter/59165

Firefox:

Object { powers: (4) […], model: {…}, submitted: false }
Object { id: 42, name: "", power: "", alterEgo: undefined }

Chrome:

HeroFormComponent {powers: Array(4), model: Hero, submitted: false}
Hero {id: 42, name: "", power: "", alterEgo: undefined}

Comment 13

[:Harald Kirschner :digitarald]

Logan, I'd love to hear your thoughts on the scope of fixing this, please. Given the comment 4 and comment 7 – do we need JS team help or is there some intermediate step that makes this work for more cases.

Comment 14

[Oriol Brufau [:Oriol]]

(In reply to :Harald Kirschner :digitarald from comment #7)

Not sure what would change the prototype, do you have an example of what use case would miss?

Here you have an example:

function Foo(){}
var f = new Foo();
f.constructor.name; // "Foo"
f; // Foo {} in Chromium

Object.defineProperty(Foo, "name", {value: "Bar"});
f.constructor.name; // "Bar"
f; // Foo {} in Chromium

Object.setPrototypeOf(f, null);
f.constructor.name; // TypeError
f; // Foo {} in Chromium

Chromium keeps showing Foo {} even after changing Foo.name or f.[[Prototype]]. So the constructor name is preserved as an invariant, that's good (though I still prefer to see the internal JSClass).

But the proposed instance.constructor.name utterly fails in these simple cases, without even starting to consider getters with side-effects, proxies, and nastier things.

Comment 15

[:Harald Kirschner :digitarald]

Thanks for the additional examples.

Just to assess the trade offs of our options: What's the use case of the ladder 2 examples? Are you aware of frameworks/libraries depending on these patterns?

Comment 16

[Oriol Brufau [:Oriol]]

I don't know, I don't like frameworks/libraries :)
But if this change is done, I think it should behave consistently as in Chromium.
Implementing something half-broken will end up causing confusion and frustration IMO.
So I would ask the JS team, yes.

Comment 17

[Logan Smyth [:loganfsmyth]]

The logic I'd propose would be something along the lines of

const obj = /* ... */;
if (isProxy(obj)) {
  return "Proxy";
}
// Default to the object's string tag.
let name = obj[Symbol.toStringTag];
if (typeof name !== "string") {

  // Otherwise try to use the constructor name
  let constructorFn = Reflect.get(Object.getPrototypeOf(obj), "constructor", obj);
  if (constructorFn && constructorFn.name) {
    name = constructorFn.name;
  }
}

// If it's a builtin, it would probably match the above checks, but you never know if someone
// might have nulled out the object's prototype or something and we can at least try to show
// a useful name in those cases.
if (typeof name !== "string" && isBuiltinType(obj)) {
  name = builtinTypeName(obj);
}

Given that 'toStringTag' would then already be a trivial way for user code to change the tag, I don't see a problem with also using obj.constructor.name directly and using whatever value is there at runtime, but if we want to explore using the original name for the function that's probably also fine, but we'd still just run right into the same issue again because user code can also reassign constructor itself (changing the constructor also affects the name in Chrome).

Comment 18

[valentinium]

@loganfsmyth Since that proposed logic involves several MOP ops which are “unsafe” even when the obj is not a proxy, I don’t understand why proxies would be special-cased. If the solution must be side-effect free, special-casing proxies makes sense, but in that case, it appears one cannot perform the Get for "constructor" or "name" properties for the same reason (unless it’s implied that there’s some kind of engine level introspection available here to bail in certain cases?).

Comment 19

[Logan Smyth [:loganfsmyth]]

I special-cased Proxy primarily under the assumption that we'd want our preview to make it clear that the object is a proxy and not the actual proxy target.

You're right about side effects though, and that's something we'll have to take into account, both by excluding proxies and by having every property access only work if the object is not a proxy and if the property descriptor is a not an accessor descriptor.

Longer term it's possible that we could use our eager-evaluation backend to support executing sideeffect-free getters, but that would be a larger project.

Comment 20

[:Harald Kirschner :digitarald]

Logan, thanks a lot for drafting out how this could be solved. How confident would you be that this approach works for most instances (React, Angular and other common framework patterns)? Is there any other risky assumption to test out or input we need to gather – or would the next step to prioritize this to prototype?

Comment 21

[Logan Smyth [:loganfsmyth]]

Talked about this offline. I think what I have would be relatively safe. I think the main thing I'd want to block on is https://bugzilla.mozilla.org/show_bug.cgi?id=1625069 because I'd probably want to perform all of the property accesses I described using instant-eval so that they'd bail if they had sideeffects.

Comment 22

[Oriol Brufau [:Oriol]]

I still think that showing Array for a non-array like Object.create(Array.prototype) or new function Array(){} will just add confusion.
Maybe, for builtin constructors, their name should only appear if the object is an actual instance. Otherwise show the internal class, or quote the name or something.

Comment 26

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 3 duplicates.
:nchevobbe, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 27

[Alexandre Poirot [:ochameau]]

I'm wondering if we could rely on Xraywrappers to retrieve the original constructor name?

When opening these two documents in two tabs:
data:text/html,<script>function Foo(){}; var f = new Foo(); Object.defineProperty(Foo, "name", {value: "Bar"});</script>
data:text/html,<script>function Foo(){}; var f = new Foo(); Object.setPrototypeOf(f, null);</script>

Then, using the Multiprocess Browser Toolbox, pick the content process for these two data: URI tabs and evaluating:
tabs[0].content.wrappedJSObject.f.constructor.name returns Foo
tabs[1].content.wrappedJSObject.f.constructor.name returns constructor is undefined

Overriding looks safe thanks to Xrays... but nullifying the prototype isn't.

Note that Cu.unwaiveXrays(tabs[1].content.wrappedJSObject.f).constructor.name returns Object which doesn't help.
Same with ChromeUtils.getClassName(tabs[0].content.wrappedJSObject.f).
This is based on JS::GetClass(obj)->name which is most likely equivalent to DebuggerObject.getClassName:
https://searchfox.org/mozilla-central/rev/3b707c8fd7e978eebf24279ee51ccf07895cfbcb/js/src/debugger/Object.cpp#1663-1671
https://searchfox.org/mozilla-central/rev/3b707c8fd7e978eebf24279ee51ccf07895cfbcb/js/src/vm/JSObject-inl.h#533-540

Now I'm wondering if this JS_GetConstructor method could help:
https://searchfox.org/mozilla-central/rev/3b707c8fd7e978eebf24279ee51ccf07895cfbcb/js/src/jsapi.cpp#1645-1660
But It sounds unlikely as the proto will most likely be undefined or lost.