Object.keys throws for uninitialized module namespace object

[jdalton]

Found in Node here and filed as a V8 bug here. It looks like Object.keys will throw an error when provided an uninitialized module namespace object while Object.getOwnPropertyNames won't.

Comments from the V8 issue:

so i was talking with ljharb about this and we came to a different conclusion about this behaviour.

Since Object.keys (and Reflect.ownKeys, for..in, all the things broken by this upgrade) don't actually expose the value, they shouldn't throw. basically everything that doesn't expose the value, or a getter/setter, should work. anything that's static and won't change after evaluation should be always accessible.

And

See https://tc39.github.io/ecma262/#sec-module-namespace-exotic-objects.

[[GetOwnProperty]] throws because [[Get]] throws:

Let value be ? O.[[Get]](P, O).

[[Get]] throws because GetBindingValue throws:

Return ? targetEnvRec.GetBindingValue(binding.[[BindingName]], true).

GetBindingValue throws because the binding, which exists, is still uninitialized:
https://tc39.github.io/ecma262/#sec-module-environment-records-getbindingvalue-n-s

The bug is that Object.keys should not error just as Object.getOwnPropertyNames does not error.

\cc @ljharb

[ljharb]

Thanks for filing!

imo this is clearly a spec bug; any reflection operations on a TDZ-d module namespace object that solely expose keys, or enumerability/configurability/writable - not values (or thus the whole descriptor) - should work at all times.

cc @allenwb to confirm this is a spec bug; @caridy @dherman @bterlson for their thoughts.

[ljharb]

Thanks for filing!

imo this is clearly a spec bug; any reflection operations on a TDZ-d module namespace object that solely expose keys, or enumerability/configurability/writable - not values (or thus the whole descriptor) - should work at all times.

cc @allenwb to confirm this is a spec bug; @caridy @dherman @bterlson for their thoughts.

[GeorgNeis]

Besides Object.keys, this affects:

• for-in
• Object.prototype.propertyIsEnumerable
• Object.prototype.hasOwnProperty
• Object.isFrozen
• Object.isSealed
• Object.freeze
• Object.seal (assuming #858 will land)

And perhaps I missed some others.

[GeorgNeis]

Besides Object.keys, this affects:

• for-in
• Object.prototype.propertyIsEnumerable
• Object.prototype.hasOwnProperty
• Object.isFrozen
• Object.isSealed
• Object.freeze
• Object.seal (assuming #858 will land)

And perhaps I missed some others.

[caridy]

This is tricky! Spec looks good, at least in principle:

HasOwnProperty (7.3.11, which is a global abstract operation) relies on [[GetOwnProperty]] internal method (which for NS objects in specified in 9.4.6.4), which relies on [[Get]] internal method (which for NS objects in specified in 9.4.6.7) to get the current value needed for the descriptor. It just happen that the [[Get]] internal method is throwing when that binding is in TDZ. This behavior is not specific for modules, this is the regular behavior for all Objects.

Do we really want to change this behavior? If we change this to make [[Get]] to return undefined when the value is in TDZ, or even if we change upstream in [[GetOwnProperty]] to return value undefined in the descriptor when in TDZ, this has other problems (imagine that you're exporting a const whose descriptor value is sometimes undefined or the actual value, and all that is observed by the importer. I don't think those two options are real options.

Likewise, changing the regular mechanism that govern HasOwnProperty (7.3.11) is probably not an option, because it is not backward compatible. I don't see a path forward here.

[caridy]

This is tricky! Spec looks good, at least in principle:

HasOwnProperty (7.3.11, which is a global abstract operation) relies on [[GetOwnProperty]] internal method (which for NS objects in specified in 9.4.6.4), which relies on [[Get]] internal method (which for NS objects in specified in 9.4.6.7) to get the current value needed for the descriptor. It just happen that the [[Get]] internal method is throwing when that binding is in TDZ. This behavior is not specific for modules, this is the regular behavior for all Objects.

Do we really want to change this behavior? If we change this to make [[Get]] to return undefined when the value is in TDZ, or even if we change upstream in [[GetOwnProperty]] to return value undefined in the descriptor when in TDZ, this has other problems (imagine that you're exporting a const whose descriptor value is sometimes undefined or the actual value, and all that is observed by the importer. I don't think those two options are real options.

Likewise, changing the regular mechanism that govern HasOwnProperty (7.3.11) is probably not an option, because it is not backward compatible. I don't see a path forward here.

[jdalton]

I don't see a path forward here.

Special case for namespace objects in relevant methods since its own property keys are known up-front without needing to dig deeper.

[jdalton]

I don't see a path forward here.

Special case for namespace objects in relevant methods since its own property keys are known up-front without needing to dig deeper.

[ljharb]

I think a non-observable spec refactoring could absolutely allow special-cased internal methods on namespace objects - or, could even allow this to be “fixed” with fewer special cases (since the tdz is already a special case)

[ljharb]

I think a non-observable spec refactoring could absolutely allow special-cased internal methods on namespace objects - or, could even allow this to be “fixed” with fewer special cases (since the tdz is already a special case)

[erights]

A test in general we should apply every time we design the behavior of an exotic object: How faithfully can this behavior be emulated by a Proxy or membrane? I don't foresee a problem here, but we need to check.

[erights]

A test in general we should apply every time we design the behavior of an exotic object: How faithfully can this behavior be emulated by a Proxy or membrane? I don't foresee a problem here, but we need to check.

[zenparsing]

If I'm understanding correctly:

1. Object.keys has to pull out a property descriptor in order to determine whether the property is enumerable or not.
2. But it can't pull out a property descriptor without also pulling out the value of the property (assuming a data property).
3. Module namespace objects return data property descriptors.
4. In general, we aren't allowed to observe the value of an "uninitialized" binding. (That's the whole point of the TDZ.)

Taking these points together, I'm not sure how we can change Object.keys to bypass the value lookup without also bypassing the MOP (which we should not do).

Perhaps moduleNamespeceObject.[[GetOwnProperty]] could in principle have returned accessor descriptors (with get/set), but I haven't thought through the implications.

[zenparsing]

If I'm understanding correctly:

1. Object.keys has to pull out a property descriptor in order to determine whether the property is enumerable or not.
2. But it can't pull out a property descriptor without also pulling out the value of the property (assuming a data property).
3. Module namespace objects return data property descriptors.
4. In general, we aren't allowed to observe the value of an "uninitialized" binding. (That's the whole point of the TDZ.)

Taking these points together, I'm not sure how we can change Object.keys to bypass the value lookup without also bypassing the MOP (which we should not do).

Perhaps moduleNamespeceObject.[[GetOwnProperty]] could in principle have returned accessor descriptors (with get/set), but I haven't thought through the implications.

[ljharb]

@zenparsing Object.keys pulls out the descriptor record - it shouldn't be accessing the record that throws, it should be reifying the value or the getter/setter into a descriptor object.

[ljharb]

@zenparsing Object.keys pulls out the descriptor record - it shouldn't be accessing the record that throws, it should be reifying the value or the getter/setter into a descriptor object.

[allenwb]

This is not a spec. bug, as such. Rather it is a direct consequence of the design of the MOP and how we choose to implement the MOP API for module namespace objects. The MOP itself is not really changeable as it is the foundation for how proxy objects and all other exotic objects interface with ES language features and the built-in library.

The MOP defines the API that exotic objects can implement in order to support such features. But exotic objects are not required to support the full functionality of ordinary objects. Throwing is a valid way to implement most of the MOP API.

For ES6 we discussed at a TC39 meeting how much of the MOP API that module namespace objects should usefully support. The consensus was that they were very exotic objects that existed to support a single function --dotted access to module imported bindings -- and that all they really needed to support was [[Get]]. That was pretty much what we did in ES 6. Most MOP operations on module namespace objects threw, or otherwise returned an error state Subsequent, people have tried to do things other than get property access with them and reported the failures as bugs. Even though the failures were "by design" some of them were fixed by adding additional functionality to the module namespace MOP implementation. But, they still are very exotic and can't be expected to be fully equivalent to ordinary objects.

It doesn't bother me that Object.keys doesn't work on them as they weren't intended to be a module reflection mirror, But here is what you might do to "fix" the problem: The dotted property access functionality is provided by [[Get]] and is not dependent upon [[GetOwnProperty]]. Subject to the essential invariants,
[[GetOwnProperty]] only provides a temporal snapshot of the state of a property.. So, make it return undefined for properties whose associated binding is uninitialized at the time the prop descriptor is constructed.

[allenwb]

This is not a spec. bug, as such. Rather it is a direct consequence of the design of the MOP and how we choose to implement the MOP API for module namespace objects. The MOP itself is not really changeable as it is the foundation for how proxy objects and all other exotic objects interface with ES language features and the built-in library.

The MOP defines the API that exotic objects can implement in order to support such features. But exotic objects are not required to support the full functionality of ordinary objects. Throwing is a valid way to implement most of the MOP API.

For ES6 we discussed at a TC39 meeting how much of the MOP API that module namespace objects should usefully support. The consensus was that they were very exotic objects that existed to support a single function --dotted access to module imported bindings -- and that all they really needed to support was [[Get]]. That was pretty much what we did in ES 6. Most MOP operations on module namespace objects threw, or otherwise returned an error state Subsequent, people have tried to do things other than get property access with them and reported the failures as bugs. Even though the failures were "by design" some of them were fixed by adding additional functionality to the module namespace MOP implementation. But, they still are very exotic and can't be expected to be fully equivalent to ordinary objects.

It doesn't bother me that Object.keys doesn't work on them as they weren't intended to be a module reflection mirror, But here is what you might do to "fix" the problem: The dotted property access functionality is provided by [[Get]] and is not dependent upon [[GetOwnProperty]]. Subject to the essential invariants,
[[GetOwnProperty]] only provides a temporal snapshot of the state of a property.. So, make it return undefined for properties whose associated binding is uninitialized at the time the prop descriptor is constructed.

[domenic]

I've always thought they would be better off as getter/setters from the mental model perspective. This kind of discussion makes me realize the ergonomics would be better too. I believe the objection was that this would allow extracting the getter/setter functions and trying to use them, which was annoying to spec/implement.

[domenic]

I've always thought they would be better off as getter/setters from the mental model perspective. This kind of discussion makes me realize the ergonomics would be better too. I believe the objection was that this would allow extracting the getter/setter functions and trying to use them, which was annoying to spec/implement.

[allenwb]

Yes, module name space objects were designed such that it would be fairly easy to for an implementation to compile mod.foo into a direct reference to the binding exported by "mod". That was really why we didn't want to expose getter functions.

[allenwb]

Yes, module name space objects were designed such that it would be fairly easy to for an implementation to compile mod.foo into a direct reference to the binding exported by "mod". That was really why we didn't want to expose getter functions.

[GeorgNeis]

I think making [[GetOwnProperty]] return undefined for such properties will result in equally surprising behavior. For instance, it would become possible that Object.keys first returns an empty array but later returns a non-empty array, and all the while Object.isFrozen returns true.

[GeorgNeis]

I think making [[GetOwnProperty]] return undefined for such properties will result in equally surprising behavior. For instance, it would become possible that Object.keys first returns an empty array but later returns a non-empty array, and all the while Object.isFrozen returns true.

[ljharb]

@GeorgNeis that should not be possible since the names are all known at the time of the object being available to user code - the only thing that would change over time is the values of the properties (and i think that any attempt to get the values should throw when in a tdz)

[ljharb]

@GeorgNeis that should not be possible since the names are all known at the time of the object being available to user code - the only thing that would change over time is the values of the properties (and i think that any attempt to get the values should throw when in a tdz)

[GeorgNeis]

Please see https://tc39.github.io/ecma262/#sec-enumerableownpropertynames.

[GeorgNeis]

Please see https://tc39.github.io/ecma262/#sec-enumerableownpropertynames.

[GeorgNeis]

Maybe there's a misunderstanding here. I assumed the suggestion is to return undefined, perhaps you assumed the suggestion is to return a property descriptor whose [[Value]] component is undefined.

[GeorgNeis]

Maybe there's a misunderstanding here. I assumed the suggestion is to return undefined, perhaps you assumed the suggestion is to return a property descriptor whose [[Value]] component is undefined.

[ljharb]

@GeorgNeis yes, i understand that that’s how the current spec operates, but that’s calling an un observable internal method. This issue suggests that we could call GetOwnProperty and enumerate the keys even if the values are not yet available.

[ljharb]

@GeorgNeis yes, i understand that that’s how the current spec operates, but that’s calling an un observable internal method. This issue suggests that we could call GetOwnProperty and enumerate the keys even if the values are not yet available.

[GeorgNeis]

But the issue lies deeper than Object.keys, so just changing Object.keys will not be enough. See the list in my earlier post. And it's not clear to me what to do about some of the others.

[GeorgNeis]

But the issue lies deeper than Object.keys, so just changing Object.keys will not be enough. See the list in my earlier post. And it's not clear to me what to do about some of the others.

[ljharb]

I agree it’s tricky.

However, i feel like it should be achievable to allow the keys-only APIs to work while continuing to throw on the APIs that reference get/set/value.

[ljharb]

I agree it’s tricky.

However, i feel like it should be achievable to allow the keys-only APIs to work while continuing to throw on the APIs that reference get/set/value.

[devsnek]

would this be a proper fix for this issue?

diff --git a/spec.html b/spec.html
index 9797576..d71e3b1 100644
--- a/spec.html
+++ b/spec.html
@@ -8574,10 +8574,10 @@
         <h1>[[OwnPropertyKeys]] ( )</h1>
         <p>When the [[OwnPropertyKeys]] internal method of a module namespace exotic object _O_ is called, the following steps are taken:</p>
         <emu-alg>
-          1. Let _exports_ be a copy of _O_.[[Exports]].
-          1. Let _symbolKeys_ be ! OrdinaryOwnPropertyKeys(_O_).
-          1. Append all the entries of _symbolKeys_ to the end of _exports_.
-          1. Return _exports_.
+          1. Let _keys_ be a copy of _O_.[[Exports]].
+          1. For each own property key _P_ of _O_ that is a Symbol, in ascending chronological order of property creation, do
+            1. Add _P_ as the last element of _keys_.
+          1. Return _keys_.
         </emu-alg>
       </emu-clause>

cc @allenwb @domenic @GeorgNeis

[devsnek]

would this be a proper fix for this issue?

diff --git a/spec.html b/spec.html
index 9797576..d71e3b1 100644
--- a/spec.html
+++ b/spec.html
@@ -8574,10 +8574,10 @@
         <h1>[[OwnPropertyKeys]] ( )</h1>
         <p>When the [[OwnPropertyKeys]] internal method of a module namespace exotic object _O_ is called, the following steps are taken:</p>
         <emu-alg>
-          1. Let _exports_ be a copy of _O_.[[Exports]].
-          1. Let _symbolKeys_ be ! OrdinaryOwnPropertyKeys(_O_).
-          1. Append all the entries of _symbolKeys_ to the end of _exports_.
-          1. Return _exports_.
+          1. Let _keys_ be a copy of _O_.[[Exports]].
+          1. For each own property key _P_ of _O_ that is a Symbol, in ascending chronological order of property creation, do
+            1. Add _P_ as the last element of _keys_.
+          1. Return _keys_.
         </emu-alg>
       </emu-clause>

cc @allenwb @domenic @GeorgNeis

[GeorgNeis]

No, this would not be a proper fix, because the fundamental issue is not about [[OwnPropertyKeys]].

[GeorgNeis]

No, this would not be a proper fix, because the fundamental issue is not about [[OwnPropertyKeys]].

[ljharb]

@GeorgNeis what do you suggest?

[ljharb]

@GeorgNeis what do you suggest?