Editorial: GetOption reform

[ptomato]

Discussion: #480

[ljharb]

i'm kind of confused why ObjectCreate is/was used here - does the spec mutate the options object?

[ptomato]

I was trying to preserve the previous behaviour, so I don't know the answer to this; maybe because the subsequent Gets would otherwise be observable?

[gibson042]

ToDateTimeOptions does indeed perform some writes, and the consequences are unnecessarily weird.

let observing = true;
const wrappers = new Set();
const options = new Proxy({}, {
  get(target, prop, receiver){
    if(observing) console.log("get:", prop, (
      receiver === options ?
        "@options" :
      Object.getPrototypeOf(receiver) === options ?
        "@wrapper" + [...wrappers.add(receiver)].indexOf(receiver) :
      receiver
    ));
    let val = target[prop];
    Reflect.defineProperty(target, prop, {
      enumerable: true,
      get(){ return val },
      set(v){
        if (observing) console.log("set:", prop, v, (
          this === options ?
            "@options" :
          Object.getPrototypeOf(this) === options ?
            "@wrapper" + [...wrappers.add(this)].indexOf(this) :
          this
        ));
        val = v;
        return true;
      }
    });
    return val;
  }
});

new Intl.DateTimeFormat(undefined, options);
/* =>
get: weekday @wrapper0
get: year @wrapper0
get: month @wrapper0
get: day @wrapper0
get: hour @wrapper0
get: minute @wrapper0
get: second @wrapper0
get: fractionalSecondDigits @wrapper0
get: localeMatcher @wrapper0
get: calendar @wrapper0
get: numberingSystem @wrapper0
get: hour12 @wrapper0
get: hourCycle @wrapper0
get: timeZone @wrapper0
get: dateStyle @wrapper0
get: timeStyle @wrapper0
get: weekday @wrapper0
get: era @wrapper0
get: hour @wrapper0
get: minute @wrapper0
get: second @wrapper0
get: timeZoneName @wrapper0
get: fractionalSecondDigits @wrapper0
get: formatMatcher @wrapper0
*/

observing = false;
[...wrappers].forEach((wrapper, i) => {
  console.log("wrapper" + i + ":", wrapper, Object.getPrototypeOf(wrapper));
});

// => wrapper0: {year: "numeric", month: "numeric", day: "numeric"} Proxy {…}

If we're making normative changes, I'd like to remove observable internal objects and multiple reads of the same properties.

[ljharb]

In that case, shouldn’t the object be cloned before it’s written to, instead of making an inheriting object? (something that’s observable via proxies, and a very weird thing imo to have be observable)

[gibson042]

Long-overdue issue: #706

[leobalter]

I understand the goal for consistency, but I'm slightly concern about the potential breaking changes.

If a normative change is proposed where we limit a feature we've had before, I'd like to see Test262 reflecting these in details because we need to understand the possible outcomes for a potential breaking change. With these tests I can also ask implementers if they are good with the proposed change.

Probably the easiest outcome would be a different abstract for Temporal.

[ptomato]

I've updated this with the suggestions in #480. It now does not change any observable behaviour.

If desired, subsequent normative changes could be made:

• In InitializeCollator: change to GetOptionsObject.
• In ToDateTimeOptions: get rid of the weird ObjectCreate(_options_) behaviour mentioned above, and use either CoerceOptionsToObject or GetOptionsObject. (This is now untouched in this PR.)
• In the Intl.Locale constructor: change to GetOptionsObject.
• In SupportedLocales: change to GetOptionsObject.
• In InitializeNumberFormat: change to GetOptionsObject.
• In InitializePluralRules: change to GetOptionsObject.
• In InitializeRelativeTimeFormat: change to GetOptionsObject.

(Edit 2021-01-08: renamed abstract operations)

[sffc]

Only in 262, right?

[ptomato]

I put it in here initially as per #480 (comment)

[gibson042]

The changes look good, but I think some naming and descriptions could be a little better.

[leobalter]

I'm in agreement with @gibson042's suggestions.

@ptomato please let me know if you're interesting in updating this. As those are minor edits, I can do it from here before merging.

Thanks!

[ptomato]

Thanks for the reviews. I didn't get to update this before my vacation but I intend to do it sometime this week or next, if that timeline works for you.

[leobalter]

No problem, my goal is to have those things done by January to get a good cut for the 2021 Edition.

[ptomato]

Updated.