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.

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

Support for prototype #14

Open
DanielSWolf opened this issue Oct 13, 2017 · 12 comments · May be fixed by #15
Open

Support for prototype #14

DanielSWolf opened this issue Oct 13, 2017 · 12 comments · May be fixed by #15

Comments

@DanielSWolf
Copy link

This package is great for creating debug output! I just compared it to a number of similar packages (eyes, jsenc, js-stringify, javascript-stringify, and stringify-object), and the output of object-inspect was by far the most helpful!

There is only one case where I'd love a bit more information, and that is when working with prototypes / ES2015 classes. Consider this code:

const str = require('object-inspect');

class Foo {
  constructor() {
    this.a = 1;
    this.b = 2;
    this.c = 42;
  }
}

console.log(str(new Foo()));

The output is { a: 1, b: 2, c: 42 }, so the information that this is an instance of class Foo gets lost.

I noticed that for certain built-in types like Map, you prefix the output with the class name. It would help me a lot if you could do the same for other prototypes. In my case, that would make the output Foo { a: 1, b: 2, c: 42 }.

@ljharb
Copy link
Member

ljharb commented Oct 14, 2017

It seems reasonable to have something like Foo { a: 1 } or { [Foo] a: 1 }.

Presumably, it would apply to any object that wasn't otherwise handled, that had a [[Prototype]] that was not Object.prototype or null?

For null objects, this is easy (and not what you asked about, but worth handling anyways). However, what about cross-realm objects? Say something whose inheritance chain is "object → OtherRealmObject.prototypenull" - OtherRealmObject.prototype !== Object.prototype, and checking for a grandparent of null would break on "object → CustomObject.prototypenull" - so how would you propose determining if an arbitrary object is Object.prototype, without using === Object.prototype?

@DanielSWolf
Copy link
Author

DanielSWolf commented Nov 2, 2017

Sorry for the late reply!

It seems reasonable to have something like Foo { a: 1 } or { [Foo] a: 1 }.

I prefer the syntax Foo { a: 1 }. It makes it clear that "Foo" applies to the entire object literal, not just to its first property. Plus, the syntax is similar to the object initialisation syntax offered by languages like C# and is also consistent with the way you format built-in types like Map.

Presumably, it would apply to any object that wasn't otherwise handled, that had a [[Prototype]] that was not Object.prototype or null?

Sounds reasonable to me.

For null objects, this is easy (and not what you asked about, but worth handling anyways). However, what about cross-realm objects? Say something whose inheritance chain is "object → OtherRealmObject.prototype → null" - OtherRealmObject.prototype !== Object.prototype, and checking for a grandparent of null would break on "object → CustomObject.prototype → null" - so how would you propose determining if an arbitrary object is Object.prototype, without using === Object.prototype?

You lost me here. I don't even think we have to manually inspect the prototype chain. I just came upon the MDN documentation for Object.prototype.constructor which I admit I didn't know before. From the looks of it, getting the result we want should be as easy as this (untested!) snippet:

if (!isDate(obj) && !isRegExp(obj)) {
  var constructor = obj.constructor;
  var prefix = constructor && constructor !== Object
    ? constructor.name + ' '
    : '';
  var xs = arrObjKeys(obj, inspect);
  var body = xs.length > 0
    ? '{ ' + xs.join(', ') + ' }'
    : '{}';
  return prefix + body;
}

@ljharb
Copy link
Member

ljharb commented Nov 4, 2017

First, values from another realm (like an iframe) won't have the same Object, so === Object or !== Object isn't robust enough to be useful.

Separately, the constructor property isn't reliable, and function names are configurable, so we can't ever use either.

@DanielSWolf
Copy link
Author

Thanks for the clarification! Let me think that over.

@DanielSWolf
Copy link
Author

I believe that identifying another realm's Object should be possible. If all else fails, we could always do a string comparison, assuming that no sane developer will name their class "Object". The worst that could happen is that for instances of a class called "Object" that is not Object, object-inspect would omit the class name prefix.

But I'm seeing a second problem here: How do we get a string representation of an object's prototype? The only "official" way I'm aware of is constructor.name. So if you say we cannot use that, how can we get that string?

@ljharb
Copy link
Member

ljharb commented Nov 7, 2017

Good code is robust against insane developers too; we won't do a string comparison.

constructor.name just tries to get the "name" property of the constructor function; both of which aren't robust either.

I'm not aware of any possible way to get another realm's Object just from having an object instance from that realm.

@DanielSWolf
Copy link
Author

What about the second problem I mentioned?

@ljharb
Copy link
Member

ljharb commented Nov 7, 2017

I would possibly be content to use the constructor’s name after any possibility of it being a builtin is removed, and after checking Object.prototype.toString.call (which isn’t robust for builtins in ES6, but works fine for non-builtins)

@DanielSWolf
Copy link
Author

I've created a pull request: #15. I'm sure there are some corner cases I haven't thought of, but I think it's a good start. I'd welcome your feedback!

@DanielSWolf
Copy link
Author

Thank you for your comments! I've fixed the wrong function call you spotted and answered your questions.

@DanielSWolf
Copy link
Author

@ljharb: Is there anything more I can do? I'd love to see this PR accepted.

@ljharb
Copy link
Member

ljharb commented Dec 19, 2017

I've indicated why I think the majority of this idea is just not tenable in practice; I'll give the PR another review just in case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants