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

util.inspect getters ignored on class instance #30183

Closed
75lb opened this issue Oct 30, 2019 · 9 comments
Closed

util.inspect getters ignored on class instance #30183

75lb opened this issue Oct 30, 2019 · 9 comments
Labels
console Issues and PRs related to the console subsystem. feature request Issues that request new features to be added to Node.js. util Issues and PRs related to the built-in util module.

Comments

@75lb
Copy link

75lb commented Oct 30, 2019

  • Version: v12.12.0
  • Platform: Darwin mba4.local 18.7.0 Darwin Kernel Version 18.7.0
  • Subsystem: util.inspect

getters:true works correctly on a plain object:

const util = require('util')
util.inspect.defaultOptions.getters = true

const object = {
  get test () {
    return 'test'
  }
}
console.log(object)

Output:

{ test: [Getter: 'test'] }

But the same operation on a class instance fails to print the getter.

class Something {
  get test () {
    return 'test'
  }
}

const something = new Something()
console.log(something)

Output:

Something {}

The output I expect to see is:

Something { test: [Getter: 'test'] }
@devsnek
Copy link
Member

devsnek commented Oct 30, 2019

class methods and accessors are nonenunerable.

@Hakerh400
Copy link
Contributor

But the same operation on a class instance fails to print the getter

There are two reasons why it does not work for class instances: 1) getters are defined on the class's prototype, not on the instance itself and 2) they are not enumerable properties. You can solve it by defining the getter on the instance instead of the prototype:

class Something{
  constructor(){
    Object.defineProperty(this, 'test', {
      get(){ return 'test'; },
      enumerable: true,
    });
  }
}
console.log(util.inspect(new Something, {getters: true}));

@75lb
Copy link
Author

75lb commented Oct 31, 2019

Yes, I understand guys - thanks for explaining. I suppose I was expecting something like the Google Chrome behaviour.

<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <title>getters</title>
</head>
<body>
  <script>
    class Something {
      get test () {
        return 'test'
      }
    }

    const something = new Something()
    console.log(something)
  </script>
</body>
</html>

Google Chrome console output.

Screenshot 2019-10-31 at 10 19 20

@BridgeAR
Copy link
Member

BridgeAR commented Nov 1, 2019

I take this as a feature request to print prototype properties as well. I thought about adding support for this before, it's just not trivial to know what properties are "useful" and which are not. We might want to print prototype properties in case the object is not a built-in. That way all user defined properties would be visible (just manipulated built-in prototypes would not be visible).

@BridgeAR BridgeAR added console Issues and PRs related to the console subsystem. feature request Issues that request new features to be added to Node.js. util Issues and PRs related to the built-in util module. labels Nov 1, 2019
@75lb
Copy link
Author

75lb commented Nov 1, 2019

Hi, yes it's a feature request and relates specifically to the getters: true inspect option so to resolve, we only need to add prototype chain getters to the util.inspect output, not all prototype chain properties.

@BridgeAR
Copy link
Member

BridgeAR commented Nov 1, 2019

getters: true should be independent: that only indicates if the getter is called or not. It could be linked in a way to only inspect the prototype chain in case the getters option is set but I can think of useful cases where it would also be interesting to know that the prototype has getters in general.

we only need to add prototype chain getters to the util.inspect output, not all prototype chain properties.

I am not sure what exactly you mean by that. Every object has a prototype (even if it's a null prototype - it's some kind of prototype). We can either always inspect them in case it's not null or in case the object is not a built-in object (which is IMO favorable). There is another point about multiple inheritance: should the prototype be followed up to the top level or should only the first level be inspected?

I would implement it in a generic fashion to always print all prototype properties of the whole chain up to the first built-in. It should not print prototype properties of built-ins. That way all user properties from the prototype would be known. If someone would not use getters: true the getters would also return the corresponding value.

class Foo extends Map {
  get abc() {
    return 'abc'
  }
}

Foo.prototype.property = true

class Bar extends Foo {
  get xyz() {
    return 'xyz'
  }
}

const bar = new Bar()

console.log(bar)
// Bar [Map] {
//   [xyz]: [Getter],
//   [abc]: [Getter],
//   [property]: true
// }

// No Map prototype properties are visible!

util.inspect.defaultOptions.getters = true

console.log(bar)
// Bar [Map] {
//   [xyz]: [Getter: 'xyz'],
//   [abc]: [Getter: 'abc'],
//   [property]: true
// }

BridgeAR added a commit to BridgeAR/node that referenced this issue Dec 2, 2019
This is only active if the `showHidden` option is truthy.

The implementation is a trade-off between accuracy and performance.
This will miss properties such as properties added to built-in data
types.

The goal is mainly to visualize prototype getters and setters such as:

class Foo {
  ownProperty = true
  get bar() {
    return 'Hello world!'
  }
}

const a = new Foo()

The `bar` property is a non-enumerable property on the prototype while
`ownProperty` will be set directly on the created instance.

The output is similar to the one of Chromium when inspecting objects
closer. The output from Firefox is difficult to compare, since it's
always a structured interactive output and was therefore not taken
into account.

Fixes: nodejs#30183
@BridgeAR
Copy link
Member

BridgeAR commented Dec 2, 2019

@75lb PTAL at #30768. That should implement this properly.

@75lb
Copy link
Author

75lb commented Dec 5, 2019

@BridgeAR Sorry for the slow response.

we only need to add prototype chain getters to the util.inspect output, not all prototype chain properties.

I am not sure what exactly you mean by that.

After reading this I realised that I misunderstood what the getters: true flag is intended to do. You describe it to mean invoke getters, i thought it meant show getters (and the getters only, not every property type). I thought, if I set getters: true then the evaluated value of each getter in the user-prototype chain would be added to the util.inspect output (and the getter values only, not everything). That is the behaviour I was looking for.

I had a quick play with your PR - generally, it's an improvement thanks. It sort-of solves my issue but I have some feedback...

Take this trivial example.

const util = require('util')
util.inspect.defaultOptions.getters = true
util.inspect.defaultOptions.showHidden = true

class Vehicle {
  get state () {
    return 'stationary'
  }
  start () {}
}

class Car extends Vehicle {
  constructor () {
    super()
    this.fuel = 'deisel'
  }

  get windowCount () {
    return 6
  }

  openWindow () {}
}

const car = new Car()
car.stop = function () {}

console.log(car)

When run with node v13.2, neither the getters or their values are displayed.

$ node -v
v13.2.0

$ node tmp/example.js
Car {
  fuel: 'deisel',
  stop: <ref *1> [Function (anonymous)] {
    [length]: 0,
    [name]: '',
    [arguments]: null,
    [caller]: null,
    [prototype]: { [constructor]: [Circular *1] }
  }
}

When run using your PR the getters and their resolved values are displayed.

$ ./out/Release/node -v
v14.0.0-pre

$ ./out/Release/node tmp/example.js
Car {
  fuel: 'deisel',
  stop: <ref *1> [Function (anonymous)] {
    [length]: 0,
    [name]: '',
    [arguments]: null,
    [caller]: null,
    [prototype]: { [constructor]: [Circular *1] }
  },
  [windowCount]: [Getter: 6],
  [openWindow]: [Function: openWindow] { [length]: 0, [name]: 'openWindow' },
  [state]: [Getter: 'stationary'],
  [start]: [Function: start] { [length]: 0, [name]: 'start' }
}

This only issue I personally have with the new output is that it displays everything, which can lead to some pretty heavy output. For example, here is the output when I console.log a real-life object from one of my apps with getters and showHidden set to false:

Tom {
  name: 'tom',
  testFn: [Function (anonymous)],
  index: 1,
  ended: true,
  result: true,
  markedSkip: undefined,
  markedOnly: undefined,
  markedBefore: undefined,
  markedAfter: undefined,
  markedTodo: undefined,
  options: { timeout: 10000, maxConcurrency: 10 },
  stats: {
    start: 113.35162699222565,
    end: 113.47576799988747,
    duration: 0.12414100766181946,
    finish: [Function: finish]
  },
  context: TestContext { name: 'tom', index: 1, data: undefined }
}

With getters and showHidden set to true:

Tom {
  name: 'tom',
  testFn: [Function (anonymous)] { [length]: 0, [name]: '' },
  index: 1,
  ended: true,
  result: true,
  markedSkip: undefined,
  markedOnly: undefined,
  markedBefore: undefined,
  markedAfter: undefined,
  markedTodo: undefined,
  options: { timeout: 10000, maxConcurrency: 10 },
  stats: {
    start: 74.19046199321747,
    end: 74.32696598768234,
    duration: 0.13650399446487427,
    finish: [Function: finish] {
      [length]: 1,
      [name]: 'finish',
      [prototype]: [finish]
    }
  },
  context: TestContext { name: 'tom', index: 1, data: undefined },
  [toString]: [Function: toString] { [length]: 0, [name]: 'toString' },
  [group]: [Function: group] { [length]: 2, [name]: 'group' },
  [test]: [Function: test] { [length]: 3, [name]: 'test' },
  [skip]: [Function: skip] { [length]: 2, [name]: 'skip' },
  [only]: [Function: only] { [length]: 2, [name]: 'only' },
  [before]: [Function: before] { [length]: 2, [name]: 'before' },
  [todo]: [Function: todo] { [length]: 2, [name]: 'todo' },
  [after]: [Function: after] { [length]: 2, [name]: 'after' },
  [_onlyExists]: [Function: _onlyExists] { [length]: 0, [name]: '_onlyExists' },
  [_skipLogic]: [Function: _skipLogic] { [length]: 0, [name]: '_skipLogic' },
  [setState]: [Function: setState] { [length]: 3, [name]: 'setState' },
  [run]: [AsyncFunction: run] {
    [length]: 0,
    [name]: 'run',
    [Symbol(Symbol.toStringTag)]: 'AsyncFunction'
  },
  [reset]: [Function: reset] { [length]: 1, [name]: 'reset' },
  [_getPerformance]: [AsyncFunction: _getPerformance] {
    [length]: 0,
    [name]: '_getPerformance',
    [Symbol(Symbol.toStringTag)]: 'AsyncFunction'
  },
  [children]: [Getter/Setter] [ [length]: 0 ],
  [parent]: [Getter/Setter: undefined],
  [add]: [Function: add] { [length]: 1, [name]: 'add' },
  [append]: [Function: append] { [length]: 1, [name]: 'append' },
  [prepend]: [Function: prepend] { [length]: 1, [name]: 'prepend' },
  [remove]: [Function: remove] { [length]: 1, [name]: 'remove' },
  [level]: [Function: level] { [length]: 0, [name]: 'level' },
  [getDescendentCount]: [Function: getDescendentCount] {
    [length]: 0,
    [name]: 'getDescendentCount'
  },
  [tree]: [Function: tree] { [length]: 0, [name]: 'tree' },
  [root]: [Function: root] { [length]: 0, [name]: 'root' },
  [inspect]: [Function: inspect] { [length]: 1, [name]: 'inspect' },
  [parents]: [Function: parents] { [length]: 0, [name]: 'parents' },
  [Symbol(Symbol.iterator)]: [GeneratorFunction: [Symbol.iterator]] {
    [length]: 0,
    [name]: '[Symbol.iterator]',
    [prototype]: Object [Generator] {},
    [Symbol(Symbol.toStringTag)]: 'GeneratorFunction'
  },
  [state]: [Getter/Setter: undefined],
  [resetState]: [Function: resetState] { [length]: 0, [name]: 'resetState' }
}

You can see the output quickly gets very busy.

When i set getters: true, I am only interested in the getters - not everything. That is the behaviour I'm looking for. I'm not sure how that behaviour should be implemented, if at all.

@BridgeAR
Copy link
Member

BridgeAR commented Dec 6, 2019

@75lb I updated my PR to exclude functions for now, so it should pretty much do what you ask for. I think ideally, we'll have a fine grained option where it's possible to define what part users would use to see that are normally hidden.

That way it would also be possible to define that you are only interested in prototype properties (excluding functions).

The verbosity is in general a problem that can IMO only be solved in two ways: fine grained options for the user and later on a width first algorithm instead of a depth first one. The latter allows to count how many properties where already printed and limit the further inspection from that point on. It is not that trivial to implement that though.

BridgeAR added a commit that referenced this issue Dec 13, 2019
This is only active if the `showHidden` option is truthy.

The implementation is a trade-off between accuracy and performance.
This will miss properties such as properties added to built-in data
types.

The goal is mainly to visualize prototype getters and setters such as:

class Foo {
  ownProperty = true
  get bar() {
    return 'Hello world!'
  }
}

const a = new Foo()

The `bar` property is a non-enumerable property on the prototype while
`ownProperty` will be set directly on the created instance.

The output is similar to the one of Chromium when inspecting objects
closer. The output from Firefox is difficult to compare, since it's
always a structured interactive output and was therefore not taken
into account.

PR-URL: #30768
Fixes: #30183
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
MylesBorins pushed a commit that referenced this issue Dec 17, 2019
This makes sure that the regular expression matches all built-in
objects properly. So far a couple where missed.

PR-URL: #30768
Fixes: #30183
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
MylesBorins pushed a commit that referenced this issue Dec 17, 2019
This is only active if the `showHidden` option is truthy.

The implementation is a trade-off between accuracy and performance.
This will miss properties such as properties added to built-in data
types.

The goal is mainly to visualize prototype getters and setters such as:

class Foo {
  ownProperty = true
  get bar() {
    return 'Hello world!'
  }
}

const a = new Foo()

The `bar` property is a non-enumerable property on the prototype while
`ownProperty` will be set directly on the created instance.

The output is similar to the one of Chromium when inspecting objects
closer. The output from Firefox is difficult to compare, since it's
always a structured interactive output and was therefore not taken
into account.

PR-URL: #30768
Fixes: #30183
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
BridgeAR added a commit to BridgeAR/node that referenced this issue Jan 20, 2020
This makes sure that the regular expression matches all built-in
objects properly. So far a couple where missed.

PR-URL: nodejs#30768
Fixes: nodejs#30183
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
BridgeAR added a commit to BridgeAR/node that referenced this issue Jan 20, 2020
This is only active if the `showHidden` option is truthy.

The implementation is a trade-off between accuracy and performance.
This will miss properties such as properties added to built-in data
types.

The goal is mainly to visualize prototype getters and setters such as:

class Foo {
  ownProperty = true
  get bar() {
    return 'Hello world!'
  }
}

const a = new Foo()

The `bar` property is a non-enumerable property on the prototype while
`ownProperty` will be set directly on the created instance.

The output is similar to the one of Chromium when inspecting objects
closer. The output from Firefox is difficult to compare, since it's
always a structured interactive output and was therefore not taken
into account.

PR-URL: nodejs#30768
Fixes: nodejs#30183
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
MylesBorins pushed a commit that referenced this issue Jan 30, 2020
This makes sure that the regular expression matches all built-in
objects properly. So far a couple where missed.

Backport-PR-URL: #31431
PR-URL: #30768
Fixes: #30183
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
MylesBorins pushed a commit that referenced this issue Jan 30, 2020
This is only active if the `showHidden` option is truthy.

The implementation is a trade-off between accuracy and performance.
This will miss properties such as properties added to built-in data
types.

The goal is mainly to visualize prototype getters and setters such as:

class Foo {
  ownProperty = true
  get bar() {
    return 'Hello world!'
  }
}

const a = new Foo()

The `bar` property is a non-enumerable property on the prototype while
`ownProperty` will be set directly on the created instance.

The output is similar to the one of Chromium when inspecting objects
closer. The output from Firefox is difficult to compare, since it's
always a structured interactive output and was therefore not taken
into account.

Backport-PR-URL: #31431
PR-URL: #30768
Fixes: #30183
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
This makes sure that the regular expression matches all built-in
objects properly. So far a couple where missed.

Backport-PR-URL: #31431
PR-URL: #30768
Fixes: #30183
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
This is only active if the `showHidden` option is truthy.

The implementation is a trade-off between accuracy and performance.
This will miss properties such as properties added to built-in data
types.

The goal is mainly to visualize prototype getters and setters such as:

class Foo {
  ownProperty = true
  get bar() {
    return 'Hello world!'
  }
}

const a = new Foo()

The `bar` property is a non-enumerable property on the prototype while
`ownProperty` will be set directly on the created instance.

The output is similar to the one of Chromium when inspecting objects
closer. The output from Firefox is difficult to compare, since it's
always a structured interactive output and was therefore not taken
into account.

Backport-PR-URL: #31431
PR-URL: #30768
Fixes: #30183
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
console Issues and PRs related to the console subsystem. feature request Issues that request new features to be added to Node.js. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants