prototype property of Temporal types should not be writable

[justingrant]

Question about monkeypatching: do we allow users to use setPrototypeOf on Temporal objects to replace the entire prototype of our of our types? And, if so, then when internal methods create new Temporal instances, then is it expected that we'd be using the original prototype object or the user's monkeypatched prototype object? The polyfill currently does not disallow using setPrototypeOf, FWIW.

I'm asking because I was looking at this code in the polyfill and was wondering if it should instead be using the original prototype from GetIntrinsic or not:

proposal-temporal/polyfill/lib/ecmascript.mjs

Lines 1418 to 1420 in f2405e1

	CreateTemporalDate: (isoYear, isoMonth, isoDay, calendar = ES.GetISO8601Calendar()) => {
	const TemporalPlainDate = GetIntrinsic('%Temporal.PlainDate%');
	const result = ObjectCreate(TemporalPlainDate.prototype);

Should this code actually be this?

CreateTemporalDate: (isoYear, isoMonth, isoDay, calendar = ES.GetISO8601Calendar()) => {
    const TemporalPlainDatePrototype = GetIntrinsic('%Temporal.PlainDate.prototype%');
    const result = ObjectCreate(TemporalPlainDatePrototype);

[ljharb]

Object.getOwnPropertyDescriptor(Temporal.PlainDate, 'prototype') should show that it's unchangeable, just like every other builtin, so the two should be equivalent.

[justingrant]

Hmm, I'm seeing inconsistency between existing builtins. Currently the Temporal non-prod prototype (the one used for tests) is writable, like Intl.DateTimeFormat but unlike Date. Here's latest Chrome:

And latest Firefox has the same behavior:

[ljharb]

I can't speak to the Intl ones, but I would expect the 262 ones to all use the same property descriptor. If the Temporal spec somehow says something different, it should be fixed - a function's prototype property shouldn't be changeable.

[ljharb]

altho actually now that I check, a userland function does have a writable prototype property (Object.getOwnPropertyDescriptor(function () {}, 'prototype')) so now i'm not sure.

In a polyfill, if .prototype is writable, then absolutely you'd want to get the intrinsic value and not dot-chain off the constructor.

[justingrant]

Meeting 2021-12-09:

• The prototype should not be writable according to spec.
• Current polyfill lets it be writable; that's a polyfill bug.
• This is a gap in Test262 coverage, will fix that too in the PR to fix this bug.
• Intl.DateTimeFormat prototype being writable is a polyfill bug too (we replace Intl.DateTimeFormat in the polyfill)
• Polyfill code for creating instances is OK
• Let's remove the %*.prototype% intrinsics from polyfill because it's not needed (because .prototype can't be changed, it's always safe to call it on an intrinsic)

[justingrant]

• Current polyfill lets it be writable; that's a polyfill bug.
• Intl.DateTimeFormat prototype being writable is a polyfill bug too (we replace Intl.DateTimeFormat in the polyfill)

Turns out the problem isn't with the core polyfill. It's a problem with the wrapper code used to load the polyfill into the Test262 tests (it also affects the docs where you can run Temporal code in the browser console). The core polyfill code correctly sets .prototype descriptor to writable: false. Here's an example when running the Demitasse tests under the debugger:

[justingrant]

It's a problem with the wrapper code used to load the polyfill into the Test262 tests (it also affects the docs where you can run Temporal code in the browser console).

I dug into this more, and it turns out that Test262 isn't affected either, because the polyfill code run by Test262 is not transpiled . The root cause is how Babel transpiles the ES class keyword into pre-ES6 code. Repro:

// Babel transpiles `class` into simple functions
var babelClass = function Foo() {};
Object.getOwnPropertyDescriptor(babelClass, 'prototype').writable;
// => true

var es6Class = class Bar {};
Object.getOwnPropertyDescriptor(es6Class, 'prototype').writable;
// => false

Babel issue (open since 2015): babel/babel#2025
Babel WIP PR: babel/babel#12115

For the non-prod polyfill in the browser playground it's relatively straightforward to work around this in the wrapper code that loads the polyfill into the playground. For prod pollyfills, we may need to figure out a more general solution, or only transpile to ES6+, or perhaps just live with it.

Given above, is it still worth adding Test262 tests to verify that the prototype's descriptor is writable: false ?

[ljharb]

test262 should certainly be verifying it, otherwise browsers etc might implement it wrong.

[justingrant]

test262 should certainly be verifying it, otherwise browsers etc might implement it wrong.

tc39/test262#3344

Babel WIP PR: babel/babel#12115

Looks like there's been a lot of progress on this PR today. Thanks @nicolo-ribaudo and @ljharb!

[justingrant]

I just pushed #1974 to fix this issue. Note that the polyfill's Intl.DateTimeFormat implementation had a writeable prototype regardless of Babel or not; it's also fixed in the same PR.