Abstract operation for extracting key-value entries

[bathos]

Just wanted to log an issue for this so it doesn’t disappear:

Since Object.fromEntries shares some logic with the Map constructor, it was suggested that introducing a common abstract operation and updating the Map constructor accordingly would be helpful for clarity / consistency.

[zloirock]

I'm not sure that it's really required - it can more confusing the spec. This logic already the same at least for Map and WeakMap. It could be refactored later, already after adding to the spec.

[ljharb]

I think it would make the spec less confusing to not have steps repeated, and it would help implementations know with certainty that the code paths should be the same. Especially since it would end up in three places - Map, WeakMap, and this proposal - it should absolutely be a separate abstract operation, as part of this proposal.

[bakkot]

This could reasonably be done as part of the stage 4 PR. It's not totally trivial, though:

I just spent a little while working through some of the details, one of which is that ToPropertyKey is observable and so it matters when it's performed. I think the order currently specified (which I also just tweaked, so retrieving the value happens before coercing the key) is the most correct, but it implies that ToPropertyKey happens in the middle of walking through the iterator.

So any such abstract operation would need some way of handing ToPropertyKey in the middle, such as a boolean flag which would control whether the relevant steps were performed and the coerced key returned or if the key was to be returned uncoerced.

[bathos]

Regarding the evaluation order, I’d figured key coercion should take place before the value is accessed by (loose) analogy with the most similar existing operation I could think of, computed property evaluation:

void { [{ toString() { throw 1; } }]: { get x() { throw 2; } }.x } // throws 1, not 2

I have no real opinion on this, but wanted to supply the rationale for the previous ordering.

[bakkot]

Ah, thanks, I hadn't thought of that. (TC39 is actually considering changing this, btw, though it wouldn't affect that case.) I was thinking of the computed property assignment case; see description there.

I confess part of my motivation was that it simplifies the polyfill substantially.

[bakkot]

As another data point for the evaluation order question, the WeakMap constructor has a typecheck on keys (to ensure they're Objects), and that typecheck happens after getting the value out of the record. So I think ToPropertyKey should also happen after getting getting the value out of the record.

(The constructor is defined in terms of WeakMap.prototype.set, which is where the actual typecheck happens.)

Now that I've looked, I notice that the WeakMap constructor actually does an observable invocation of the set property of the object being constructed in the middle of the loop, so that the flag above wouldn't be enough. You'd have to have spec operations as first class objects in spec land, so that you could pass a series of algorithm steps like "k. Let propertyKey be ToPropertyKey(key) [...] n. Peform ! CreateDataPropertyOrThrow(obj, propertyKey, value)." or "i. Let status be Call(adder, map, « k.[[Value]], v.[[Value]] »). j. If status is an abrupt completion, return ? IteratorClose(iteratorRecord, status)." to another algorithm.

[ljharb]

I've filed tc39/ecma262#1262 which creates this abstract operation; please review.

[bakkot]

@ljharb Do you have a version of this proposal's spec text which makes use of that abstract operation?

[ljharb]

@bakkot I've pushed up https://github.com/tc39/proposal-object-from-entries/compare/add_entries_from_iterable which should cover it.

[bakkot]

@ljharb Nice! I don't think the definition for CreateDataProperty Functions is quite right, though: I would expect it to be

-<p>A CreateDataProperty function is an anonymous built-in function. When a CreateDataProperty function is called with arguments _propertyKey_ and _value_, the following steps are taken:</p>
+<p>A CreateDataProperty function is an anonymous built-in function. When a CreateDataProperty function is called with arguments _key_ and _value_, the following steps are taken:</p>
<emu-alg>
  1. Let _obj_ be the *this* value.
  1. Assert: Type(_obj_) is Object.
  1. Assert: _obj_ is an extensible ordinary object.
-  1. Assert: IsPropertyKey(_propertyKey_) is *true*.
+  1. Let _propertyKey_ be ? ToPropertyKey(_key_).
-  1. Perform ? CreateDataPropertyOrThrow(_obj_, _propertyKey_, _value_).
+  1. Perform ! CreateDataPropertyOrThrow(_obj_, _propertyKey_, _value_).
<emu-alg>

Anyway, I think that works, though I feel pretty weird about reifying (at least in spec-land) a JS function for this - definitely it would need a big note explaining that the function and its invocation are never observable to user code.

[ljharb]

Thanks; I've updated it.