Missing RequireInternalSlot

[FrankYFTang]

The following places missing RequireInternalSlot
11.4.3 get Temporal.TimeZone.prototype.id
https://tc39.es/proposal-temporal/#sec-get-temporal.timezone.prototype.id

11.4.6 Temporal.TimeZone.prototype.getPlainDateTimeFor ( instant [ , calendarLike ] )
https://tc39.es/proposal-temporal/#sec-temporal.timezone.prototype.getplaindatetimefor

11.4.12 Temporal.TimeZone.prototype.toJSON ( )
https://tc39.es/proposal-temporal/#sec-temporal.timezone.prototype.tojson

12.4.3 get Temporal.Calendar.prototype.id
https://tc39.es/proposal-temporal/#sec-get-temporal.calendar.prototype.id

12.4.24 Temporal.Calendar.prototype.toJSON ( )
https://tc39.es/proposal-temporal/#sec-temporal.calendar.prototype.tojson

[ptomato]

I am not sure about these. At one point in the history, it was intentional that we did not do a brand check there, but I believe that was later removed. So I think this is correct, but I'll have to do some digging to know for sure.

[Ms2ger]

The goal is to support this code:

class TimeZone extends Temporal.TimeZone {
  constructor() { super("UTC"); }
  toString() { return "my own UTC variant"; }
}
(new TimeZone()).id === "my own UTC variant"

As far as I'm aware, the champions haven't changed their mind on this.

[ptomato]

But that's not affected by these brand checks? (new TimeZone()) has the correct internal slot.

I'm not sure, but I think after the big subclassing change, the only way to get into a situation where the brand check would fail, is with a plain object:

const timeZone = {
  getPossibleInstantsFor() { ... },
  getOffsetNanosecondsFor() { ... },
  toString() { return "my own UTC variant" },
};
Temporal.TimeZone.prototype.toJSON.call(timeZone) === "my own UTC variant"

[Ms2ger]

Oh, right. Maybe this would be a little more plausible:

const timeZone = Object.create(Temporal.TimeZone.prototype, {
  getPossibleInstantsFor() { ... },
  getOffsetNanosecondsFor() { ... },
  toString() { return "my own UTC variant" },
});
timeZone.toJSON() === "my own UTC variant"

[ljharb]

I’d prefer that not be supported, but i think we did have a plenary discussion on it.

[FrankYFTang]

Oh, right. Maybe this would be a little more plausible:

const timeZone = Object.create(Temporal.TimeZone.prototype, {
  getPossibleInstantsFor() { ... },
  getOffsetNanosecondsFor() { ... },
  toString() { return "my own UTC variant" },
});
timeZone.toJSON() === "my own UTC variant"

so... why is only toJSON , getPlainDateTimeFor and id need to be supported this way but not other methods in these two classes? where is the line? why that is the line? what is the marginal benefit now?

[ptomato]

I’d prefer that not be supported, but i think we did have a plenary discussion on it.

We did discuss in plenary that plain objects should be supported as valid values as far as input into Temporal methods is concerned, but I'm not sure that that requires also supporting this case, since this object without the brand isn't passed as input to a Temporal method. Here, the user is deliberately calling a method on it that isn't one of the ones that is called internally by Temporal.

I don't have a strong opinion personally about whether this case should be supported or not, but I don't think that any previous decisions constrain the outcome one way or the other.

so... why is only toJSON , getPlainDateTimeFor and id need to be supported this way but not other methods in these two classes? where is the line? why that is the line? what is the marginal benefit now?

If I understand correctly, the line would be that TimeZone methods which call other TimeZone methods would not have a brand check. (So, if we were to support this, we should actually remove the brand check from getOffsetStringFor and getInstantFor.)

[ptomato]

In the champions meeting of 2021-10-28 we decided to add brand checks in these places. Reasoning being:

• The current inconsistency is not good, either you should be able to use Function.call on all of the non-required prototype methods with a plain object as the receiver, or none of them;
• Removing the capability doesn't interfere with the general principle decided in plenary of being able to use a plain object implementing the time zone protocol everywhere a Temporal.TimeZone object would be accepted (ditto for Temporal.Calendar), because nothing in ECMAScript calls any of these non-required methods internally;
• If we added the brand checks now, they could later be removed in a web-compatible way.*

* As long as there was still at least one Temporal.TimeZone/Calendar method with a brand check, assuming codehag/documenting-invariants#13 finds broad support. The required methods would always have to have brand checks, so that wouldn't be a problem.