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.

[GeorgNeis]

Besides Object.keys, this affects:

• for-in
• Object.prototype.propertyIsEnumerable
• Object.prototype.hasOwnProperty
• Object.isFrozen
• Object.isSealed
• Object.freeze
• Object.seal (assuming Relax [[DefineOwnProperty]] on module namespace exotic objects. #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.

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

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

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

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

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