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()ing a Proxy [sometimes] shows defined properties as undefined #21639

Closed
broofa opened this issue Jul 3, 2018 · 8 comments
Closed
Labels
util Issues and PRs related to the built-in util module.

Comments

@broofa
Copy link

broofa commented Jul 3, 2018

  • Version: v10.5.0
  • Platform: Darwin RWK-Mac-2017.local 17.3.0 Darwin Kernel Version 17.3.0: Thu Nov 9 18:09:22 PST 2017; root:xnu-4570.31.3~1/RELEASE_X86_64 x86_64
  • Subsystem: util

'Ran into this in the process of making virtual properties on a Proxy object enumerable.
See tc39/ecma262#367 (comment)

Briefly, customizing the enumeration of a Proxy object requires implementing both an ownKeys() method (to define the keys to be enumerated) and getOwnPropertyDescriptor() to declare those keys as enumerable. The problem here is that property values in Proxy objects are not canonically defined by a property descriptor the way they are with in a vanilla JS object; they're defined by the Proxy handler.get() method. This leads to util.inspect showing seemingly undefined values that, when referenced directly, are actually defined ...

> x = new Proxy({a: 1, b:2}, {
...   get(o, k) {return k == `c` ? o.a + o.b : o[k];},
...   ownKeys(o) {return [...Object.keys(o), 'c']},
...   getOwnPropertyDescriptor(o, k) {return {configurable: true, enumerable: true}},
... })
Proxy [ { a: 1, b: 2 },
  { getOwnPropertyDescriptor: [Function: getOwnPropertyDescriptor],
    ownKeys: [Function: ownKeys],
    get: [Function: get] } ]
> console.log(x)
{ a: undefined, b: undefined, c: undefined }
undefined
> x.a
1
> x.b
2
> x.c
3
>
@addaleax addaleax added the util Issues and PRs related to the built-in util module. label Jul 3, 2018
@devsnek
Copy link
Member

devsnek commented Jul 3, 2018

I would consider this a bug with your proxy, not a bug with node.

@broofa
Copy link
Author

broofa commented Jul 3, 2018

A fair point (and, in fact, my workaround for this is to add value: this.get(k) to the descriptor so it's consistently defined.) But I'm going to argue that's a cop out. This problem arises because proxy property values may be ambiguously defined.

E.g. What's the correct value of foo in the following? Is it "hello" or is it "world"?

> x = new Proxy({}, {ownKeys() {return ['foo']}, get() {return 'hello';}, getOwnPropertyDescriptor() {return {configurable: true, enumerable: true, value: 'world'}}});
{ foo: 'world' }
> x.foo
'hello'

Answer: Neither? Both? It's subjective.

And because it's subjective, we should look at how these two cases are used. The property descriptor value is only available via an explicit call to Object.getOwnPropertyDescriptor(), and I would argue that comparitvly few people do that. The far more common case is to just access properties directly on the object. Ergo, that's what util.inspect should show.

@devsnek
Copy link
Member

devsnek commented Jul 3, 2018

it shouldn't matter which it uses. you're returning the wrong thing from one of them. if you don't conform to standard behaviour there can be no standard fix. changing util.inspect fixes your problem and breaks someone else's, because you both depart from expected behaviour.

@devsnek
Copy link
Member

devsnek commented Jul 3, 2018

if you want, you can add a util.inspect.custom method to your object, which will let you control what inspect returns.

@TimothyGu
Copy link
Member

This problem arises because proxy property values may be ambiguously defined.

Unfortunately this is true, and we have limited options in combating this problem. One option if you have control over the global environment is that you can tweak the default showProxy option of util.inspect which console.log uses to be true, so that Proxy [] will be displayed, but setting a custom inspection function is probably the best in this case.

In general, writing a fully consistent Proxy object is remarkably difficult, simply because of how many methods one has to override. Like @devsnek, I don't really see a good way to approach this from Node.js' end, since it's IMO unreasonable to limit the Object methods we can use for an edge case.

@broofa
Copy link
Author

broofa commented Jul 3, 2018

In general, writing a fully consistent Proxy object is remarkably difficult

Tell me about it! 😄

FWIW, it seems like the use of getOwnPropertyDescriptor predates the Proxy standard by 3-4 years. f901443 'Not sure how or why that's important, other than to suggest that perhaps the current behavior isn't all that deliberate... [ducks].

@devsnek
Copy link
Member

devsnek commented Jul 3, 2018

it is still quite deliberate

@BridgeAR
Copy link
Member

It does not look like this is something we can address properly. Using proxies has multiple side effects in a lot of code and we can not properly prevent that. So sadly it is more up to the proxy implementer to work around these things. If there is something concrete we can do, please just comment here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
util Issues and PRs related to the built-in util module.
Projects
None yet
Development

No branches or pull requests

5 participants