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

Normative: Remove [[Enumerate]] and associated reflective capabilities #367

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
8 participants
@bterlson
Copy link
Member

bterlson commented Feb 5, 2016

Summary of changes (actually quite simple so I expect I'm missing something major :-P)

  • 6.1.7.2 Removed [[Enumerate]] from the table and list of invariants
  • 9.5 Proxy Object Internal Methods and Interal Slots, [[Enumerate]] table entry removed.
  • 9.4.6.1.1 Module Namespace Object [[Enumerate]] deleted
  • 9.5.11 Proxy [[Enumerate]] deleted
  • Moved ordinary [[Enumerate]] section to end of 13.7.5 for-in, renamed to EnumerateObjectProperties.
    • Add an assert to step 1 to signal callers we expect they'll only pass objects
    • Updated informative definition of EnumerateObjectProperties
  • 13.7.5.12 for-in head evaluation calls EnumerateObjectProperties(obj) instead of obj.[[Enumerate]]
  • EnumerableOwnNames: the names are always ordered the same as EnumerateObjectProperties (since we don't have to worry about this not being enforceable with proxies anymore)
  • 26.1.5 Reflect.enumerate deleted
  • 26.3.2 module namespace object's [ @@iterator ] calls EnumerateObjectProperties instead of [[Enumerate]]
@allenwb

This comment has been minimized.

Copy link
Member

allenwb commented Feb 5, 2016

Re 26.3.2. I'd just take the guts of 9.4.6.11 and insert them into 26.3.2. There is no need to tie namespace name iteration to for-in semantics

@bterlson

This comment has been minimized.

Copy link
Member Author

bterlson commented Feb 5, 2016

@allenwb I thought about that but I'd have to call for-in if the method were applied to a non-module namespace object. This was simpler I thought, but happy to change it to something like:

  1. Let N be the this value.
  2. If N is a Module Namespace exotic object, then
    1. Let exports be the value of N's [[Exports]] internal slot.
    2. Return ! CreateListIterator(N).
  3. Else if Type(N) is Object, return ? EnumerateObjectProperties(N);
  4. Else, N is not an Object, throw a TypeError exception.
@allenwb

This comment has been minimized.

Copy link
Member

allenwb commented Feb 5, 2016

??? 26.3.2 is specifically an iterator over the names of a module name space object. It really wasn't intended to be any sort of general own property name iterator. It is just sort of a design fluke it ended up that way.

I think it would be better for it to just throw if it is applied to something that isn't a module namespace exotic object

@bterlson

This comment has been minimized.

Copy link
Member Author

bterlson commented Feb 5, 2016

Uhh, yes, that is much better. Updating the PR.

@bterlson bterlson force-pushed the bterlson:no-enumerate branch from cbd59c2 to 2a57869 Feb 5, 2016

@bterlson

This comment has been minimized.

Copy link
Member Author

bterlson commented Feb 5, 2016

Fixed, thanks @allenwb.

Normative: Remove [[Enumerate]] and associated reflective capabilities
Summary of changes:

* 6.1.7.2 Removed [[Enumerate]] from the table and list of invariants
* 9.5 Proxy Object Internal Methods and Interal Slots, [[Enumerate]] table entry removed.
* 9.4.6.1.1 Module Namespace Object [[Enumerate]] moved to 26.3.2 module namespace object [ @@iterator ]
* 9.5.11 Proxy [[Enumerate]] deleted
* Moved ordinary [[Enumerate]] section to end of 13.7.5 for-in, renamed to EnumerateObjectProperties.
  * Add an assert to step 1 to signal callers we expect they'll only pass objects
  * Updated informative definition of EnumerateObjectProperties
* 13.7.5.12 for-in head evaluation calls EnumerateObjectProperties(_obj_) instead of _obj_.[[Enumerate]]
* EnumerableOwnNames: the _names_ are always ordered the same as EnumerateObjectProperties (since we don't have to worry about this not being enforceable with proxies anymore)
* 26.1.5 Reflect.enumerate deleted
@@ -4415,8 +4398,7 @@
1. Let _desc_ be ? _O_.[[GetOwnProperty]](_key_).
1. If _desc_ is not *undefined*, then
1. If _desc_.[[Enumerable]] is *true*, append _key_ to _names_.
1. If _O_.[[Enumerate]] is the ordinary object [[Enumerate]] internal method (<emu-xref href="#sec-ordinary-object-internal-methods-and-internal-slots-enumerate"></emu-xref>), then
1. Order the elements of _names_ so they are in the same relative order as would be produced by the Iterator that would be returned if the [[Enumerate]] internal method was invoked on _O_.
1. Order the elements of _names_ so they are in the same relative order as would be produced by the Iterator that would be returned if the EnumerateObjectProperties internal method was invoked with _O_.
1. Return _names_.
</emu-alg>
<emu-note>

This comment has been minimized.

Copy link
@ljharb

ljharb Feb 6, 2016

Member

Is the note on the next line ("<p>The order of elements in the returned list is the same as the enumeration order that is used by a for-in statement.</p>" but github has trouble showing it and won't let me comment on it) still necessary? It seems implied by EnumerateObjectProperties.

This comment has been minimized.

Copy link
@bterlson

bterlson Feb 9, 2016

Author Member

Agreed, removed the note.

@annevk

This comment has been minimized.

Copy link
Member

annevk commented Feb 6, 2016

How does HTML make what is now [[Enumerate]] return CreateListIterator(« ») for WindowProxy and Location objects when they are accessed from a different origin? It seems there would no longer be a hook to do that.

@domenic

This comment has been minimized.

Copy link
Member

domenic commented Feb 6, 2016

@annevk probably the best way is to make GOPD always return enumerable: false. Combined with GetPrototypeOf returning null, that should do the trick.

<p>When the abstract operation EnumerateObjectProperties is called with argument _O_, the following steps are taken:</p>
<emu-alg>
1. Assert: Type(_O_) is Object.
1. Return an Iterator object (<emu-xref href="#sec-iterator-interface"></emu-xref>) whose `next` method iterates over all the String-valued keys of enumerable properties of _O_. The Iterator object must inherit from %IteratorPrototype% (<emu-xref href="#sec-%iteratorprototype%-object"></emu-xref>). The mechanics and order of enumerating the properties is not specified but must conform to the rules specified below.

This comment has been minimized.

Copy link
@anba

anba Feb 9, 2016

Collaborator

Do you think it makes sense to define that the return method of the iterator object is null? This should ensure that the iterator object cannot escape to user code when a break or return statement is nested in the for-in loop.

This comment has been minimized.

Copy link
@bterlson

bterlson Feb 9, 2016

Author Member

Yeah, I guess both throw and return should be null.

This comment has been minimized.

Copy link
@bterlson

bterlson Feb 9, 2016

Author Member

And actually the requirement that it MUST inherit from %IteratorPrototype% is kind of bogus since the intention is this isn't observable. Can I simply remove that line?

This comment has been minimized.

Copy link
@anba

anba Feb 10, 2016

Collaborator

Yes, you can remove that line. I'd probably also add the note "The Iterator object is never directly accessible to ECMAScript code." to underline that implementations don't actually need to create a JavaScript object.

1. Return an Iterator object (<emu-xref href="#sec-iterator-interface"></emu-xref>) whose `next` method iterates over all the String-valued keys of enumerable properties of _O_. The Iterator object must inherit from %IteratorPrototype% (<emu-xref href="#sec-%iteratorprototype%-object"></emu-xref>). The mechanics and order of enumerating the properties is not specified but must conform to the rules specified below.
</emu-alg>
<p>The iterator's `next` method processes object properties to determine whether the property key should be returned as an iterator value. Returned property keys do not include keys that are Symbols. Properties of the target object may be deleted during enumeration. A property that is deleted before it is processed by the iterator's `next` method is ignored. If new properties are added to the target object during enumeration, the newly added properties are not guaranteed to be processed in the active enumeration. A property name will be returned by the iterator's `next` method at most once in any enumeration.</p>
<p>Enumerating the properties of the target object includes enumerating properties of its prototype, and the prototype of the prototype, and so on, recursively; but a property of a prototype is not processed if it has the same name as a property that has already been processed by the iterator's `next` method. The values of [[Enumerable]] attributes are not considered when determining if a property of a prototype object has already been processed. The enumerable property names of prototype objects must be obtained as if by invoking EnumerateObjectProperties passing the prototype object as the argument. EnumerateObjectProperties must obtain the own property keys of the target object as if by calling its [[OwnPropertyKeys]] internal method. Property attributes of the target object must be obtained as if by calling its [[GetOwnProperty]] internal method.</p>

This comment has been minimized.

Copy link
@anba

anba Feb 9, 2016

Collaborator

Do we need to change "as if by calling" to "by calling" to properly define the behaviour when enumerating the properties of a proxied object?

This comment has been minimized.

Copy link
@bterlson

bterlson Feb 9, 2016

Author Member

I'm not sure this is necessary, can you expand? The difference between the two seems slight, with the former signaling that the implementation need not actually be a recursive proto-walk, but the result must look like it was a recursive proto-walk. Seems like allowing this for proxies isn't bad as long as the difference isn't observable. Probably missing something.

This comment has been minimized.

Copy link
@anba

anba Feb 10, 2016

Collaborator

"as if by calling" was added in https://bugs.ecmascript.org/show_bug.cgi?id=4107, because we need the same set of property names, but we don't want to require to call [[OwnPropertyKeys]] (or [[GetOwnProperty]]). From getify/You-Dont-Know-JS#423 (comment):

"as if" has a specific meaning in standards and is fine to use in normative text. "as if x" means: the observable results will be the same as you would get if you did x but it is ok to get those results some other way.

The last part ("get those results some other way") is clearly not applicable for proxy objects (and we don't want to mislead implementers to do something other than calling [[OwnPropertyKeys]] on proxies), so it seems wrong to me to leave it in.

This comment has been minimized.

Copy link
@bterlson

bterlson Feb 12, 2016

Author Member

Ok I agree, will remove all as-ifs. Implementations must invoke [[OwnPropertyKeys]] once per object in the prototype chain and [[GetOwnProperty]] once for each property, in some order.

ljharb added a commit to tc39/proposal-object-values-entries that referenced this pull request Feb 10, 2016

@bterlson bterlson closed this in d96e60a Feb 12, 2016

@allenwb

This comment has been minimized.

Copy link
Member

allenwb commented Feb 12, 2016

except that I think you should have an "as if" in "... of prototype objects must be obtained by invoking EnumerateObjectProperties...".

It should probably "... of prototype objects must be obtained as if by performing ..."

We want to required the MOP operation calls, but should suggest that creation of intermediate iterators are required.

@bterlson

This comment has been minimized.

Copy link
Member Author

bterlson commented Feb 12, 2016

We want to required the MOP operation calls, but should suggest that creation of intermediate iterators are required.

I think you mean NOT required? I think the note in EnumerateObjectProperties that says that the iterator is never exposed to ECMAScript code makes that clear. But I can add back the first "as if by" if it helps.

@broofa

This comment has been minimized.

Copy link

broofa commented Jun 30, 2018

My apologies if this isn't appropriate here, but I'm not finding answers elsewhere on the web. With Proxy#enumerate no longer available, how does one define the enumerable properties of a proxy object?

Specifically, I have a case where I have a Proxy object that stores values on an internal Object. It'd be nice if console.log(proxyObject) showed the values in question, rather than just "{}" (empty object), but it's not clear there's a way to do that. :-/

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Jun 30, 2018

@broofa you can use the ownKeys proxy trap for that.

@broofa

This comment has been minimized.

Copy link

broofa commented Jun 30, 2018

@ljharb Hmm... that's not what I'm seeing. Am I doing something wrong, or are Node et al misinterpreting the spec?

> x = new Proxy({}, {ownKeys() {return ['a', 'b', 'c']}, get() {return 'foo'}, has() {return true}})
{}
> 'a' in x
true
> for (k in x) console.log(k)
undefined
> console.log(x)
{}
undefined
> Object.keys(x)
[]
@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Jun 30, 2018

I believe you also need to define a getOwnPropertyDescriptor trap.

@broofa

This comment has been minimized.

Copy link

broofa commented Jul 2, 2018

@ljharb Thanks, that works!

I'm leaving a breadcrumb here for what I ended up with in case anyone else stumbles across this, and also because of a possible(?) spec issue described below. If this is getting spammy, my apologies, and let me know where I should move this conversation to.

So, example of adding a virtual "c" property to a proxy object:

> 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, value: this.get(o, k)}},
... })
{ a: 1, b: 2, c: 3 }

BTW, it's worth noting that I had to set the descriptor value: this.get(o, k) to work around an inconvenient problem: Defining property descriptors means proxy properties are now defined independently in two places: handler.get(), and propertyDescriptor.value. Without a descriptor value the values can be either defined or undefined depending on how they're referenced. For example, with no descriptor value in Node ...

> 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
>

I was going to report this as a bug against node's util.inspect() method, but I'm not convinced the current behavior is wrong. If you Object.defineProperty() a value on a vanilla Object (i.e. outside a proxy), the property descriptor value is the canonical value for that object. Preferring the property descriptor value over the proxy's handler.get() value is arguably correct... or at least consistent with normal Object property descriptor behavior.

Does the Proxy spec address how implementors are expected to resolve this apparent inconsistency? FWIW, Safari and Firefox appear to use handler.get() if the descriptor value is omitted, which is kind of nice.

@claudepache

This comment has been minimized.

Copy link
Contributor

claudepache commented Jul 3, 2018

@broofa

BTW, it's worth noting that I had to set the descriptor value: this.get(o, k) to work around an inconvenient problem: Defining property descriptors means proxy properties are now defined independently in two places: handler.get(), and propertyDescriptor.value.

The [[OwnPropertyKeys]], [[GetOwnProperty]], [[Get]] and [[HasProperty]] internal methods of the object give partially redundant informations, and it is the responsibility of the writer of the Proxy handler to keep them reasonably consistent by implementing carefully, or not implementing, the corresponding traps (ownKeys, getOwnPropertyDescriptor, get and has). BTW, this is an argument for not having [[Enumerate]], because it would be one more internal method to keep consistent with others.

Does the Proxy spec address how implementors are expected to resolve this apparent inconsistency? FWIW, Safari and Firefox appear to use handler.get() if the descriptor value is omitted, which is kind of nice.

ECMAScript does not specify the behaviour of debugging facilities such as console.log or util.inspect.

@broofa

This comment has been minimized.

Copy link

broofa commented Jul 3, 2018

ECMAScript does not specify the behaviour of debugging facilities such as console.log or util.inspect.

My point was only mainly that, in making an object enumerable, you are forced to confront this ambiguity about where values are defined. That the behavior of calling code (e.g. util.inspect) is not well defined just highlights this ambiguity.

@allenwb

This comment has been minimized.

Copy link
Member

allenwb commented Jul 3, 2018

Preferring the property descriptor value over the proxy's handler.get() value is arguably correct... or at least consistent with normal Object property descriptor behavior.

Does the Proxy spec address how implementors are expected to resolve this apparent inconsistency?

The MOP (ie Proxy) spec. allows an exotic object to produce all sorts of inconsistent results.
The only consistency requirements are those defined as essential invariants.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.