Spec review comments

[eemeli]

An initial pass for #14:

• 1.1.1 JSON.parse -- The spec currently doesn't include the term "options bag", and it's probably not worthwhile introducing it here.
• 1.1.1.3 CreateJSONParseOptions -- Why check HasProperty before calling Get? That seems unnecessary. ECMA-402 has GetOption, which seems like it might be quite useful in 262 as well.
• 1.1.1.3 CreateJSONParseOptions -- Could we throw instead of casting values to booleans?

[acutmore]

The options bag for Error does a has check before the get and this is a reasonably recent addition to the spec.

https://tc39.es/ecma262/multipage/fundamental-objects.html#sec-installerrorcause

Though Iterator.zip only does the get, and is the newer proposal.

https://tc39.es/proposal-joint-iteration/#sec-iterator.zipkeyed

[eemeli]

I can't remember, but is there maybe a reason that using { cause: undefined } has an observable effect on Errors? Here and in Iterator.zip we should definitely ignore undefined option values.

[ljharb]

There is a reason - the cause of an error can be throw undefined or throw anyValue, so, cause in options effectively results in the test of "is a cause present" being 'cause' in error rather than checking for undefined.

Does JSON have a need to differentiate undefined from "absent"? I would assume no, which means that cause isn't the model to follow.

[acutmore]

Thanks Jordan. That makes sense.

[peetklecha]

thanks @eemeli ! i’ve addressed your first two comments in #17 . doing so meant foreclosing on the third one, since GetOption in 402 coerces rather than throwing. i think this is also consistent with the convention: https://github.com/tc39/how-we-work/blob/main/normative-conventions.md#avoid-coercing-arguments-to-types-other-than-boolean

[ljharb]

Def use Temporal's GetOption, not 402's