Port commits from proposal-temporal 2023-04-25 #247

justingrant · 2023-04-25T09:12:15Z

Here's another batch of 28 porting commits. This batch catches us up to tc39/proposal-temporal#2557, which means that subsequent porting PRs can get ecmascript.mjs/.ts merge conflicts without having to use upstream's porting branch.

Also, all the expected failures (from porting, that is) are fixed! Only a few more PRs remain before we're fully caught up with upstream. This will be a good point to release to npm!

Breaking Changes

This polyfill is not yet expected to be run in production. But we know you might like a heads-up about two large breaking changes coming from upstream that are included in this PR:

Replacing `timeZone` and `calendar` properties (tc39/proposal-temporal#2482)

In Temporal.ZonedDateTime, the timeZone property was replaced by a timeZoneId property that returns the string ID and getTimeZone() method that returns a Temporal.TimeZone object. Using strings is preferred for built-in time zones.

In Temporal.ZonedDateTime and all Temporal.Plain* types, the calendar was replaced by a calendarId property that returns a string ID, and a getCalendar() method that returns a Temporal.Calendar object. Using strings is preferred for built-in calendars.

Removed `Intl.DateTimeFormat` formatting support for `Temporal.ZonedDateTime` objects (tc39/proposal-temporal#2522)

For simple cases where performance isn't critical, use toLocaleString():

zdt = Temporal.ZonedDateTime.from('2023-04-03T09:40-07:00[America/Los_Angeles]');
zdt.toLocaleString('en', {
  year: 'numeric',
  month: 'long',
  day: 'numeric',
  hour: 'numeric',
  minute:'2-digit',
  timeZoneName: 'long' 
});
// => 'April 3, 2023 at 9:40 AM Pacific Daylight Time'

For other cases like formatting to parts and/or ranges, or for maximum performance when formatting in the same time zone and calendar, you can use Temporal.ZonedDateTime.prototype.epochMilliseconds as input to Intl.DateTimeFormat formatting functions. Example:

function formatRangeZonedDateTime(zdt1, zdt2, locales, options) {
  if (zdt1.timeZoneId !== zdt2.timeZoneId) {
    throw new RangeError('Cannot format a range if time zones are different');
  }
  if (zdt1.calendarId !== zdt2.calendarId) {
    throw new RangeError('Cannot format a range if calendars are different');
  }
  const { timeZone, calendar } = zdt1;
  // Format using the time zone and calendars of the ZonedDateTime instances
  const formatter = Intl.DateTimeFormat(locales, { ...options, timeZone, calendar });
  return formatter.formatRange(zdt1.epochMilliseconds, zdt2.epochMilliseconds);
}

z1 = Temporal.ZonedDateTime.from('2023-04-03T09:40-07:00[America/Los_Angeles]');
z2 = Temporal.ZonedDateTime.from('2024-12-25T18:37-08:00[America/Los_Angeles]');

formatRangeZonedDateTime(z1, z2, 'en', { timeZoneName: 'short' })
// => '4/3/2023, PDT – 12/25/2024, PST'

Temporal.Now.timeZone() is replaced by Temporal.Now.timeZoneId() which always returns a time zone identifier string, rather than a TimeZone instance. See: #1808 UPSTREAM_COMMIT=d67db4a7731d13f9e6e2b4be4f6d10732756c06e

PlainTime now has no calendar. Removes the getter and the internal slot. Calendar annotations are now ignored in ParseTemporalTimeString. RejectObjectWithCalendarOrTimeZone still rejects PlainTime, so it is renamed to RejectTemporalLikeObject. ToTemporalCalendar rejects PlainTime explicitly instead of looking for its `calendar` property or treating it as a plain object calendar. See: #1808 Closes: #1588 UPSTREAM_COMMIT=61fbb401df7463e435b8e6f932de123fd5bc0c2d

Refactor so that Temporal objects' [[Calendar]] internal slot can now store either a string (builtin calendar) or an object (custom calendar). The .calendar getter is replaced with a .calendarId getter that always returns a string, and a .getCalendar() method that always returns a new object if the slot stored a string. The operations ToTemporalCalendarIdentifier and ToTemporalCalendarObject convert a [[Calendar]] slot value to one or the other. See: #1808 UPSTREAM_COMMIT=7cf55b9bea1f19ee5a911934e17a37a9bbf0a4c7

When creating Temporal objects with a builtin ISO 8601 calendar, we want them to be created with a string in the internal slot, not a Calendar instance. (For example, when parsing a calendar name out of a string or using ISO 8601 as a default.) Therefore we don't need the GetISO8601Calendar and GetBuiltinCalendar AOs anymore. See: #1808 UPSTREAM_COMMIT=52f3159ba4d03343e1cfdbcee1720b03e28753b8

Previously algorithms such as CalendarEquals used ToString to get the identifier of a calendar object. Instead, use ToTemporalCalendarIdentifier which observably gets the .id property of a custom calendar instead of getting and calling the .toString property. See: #1808 UPSTREAM_COMMIT=5e0e87061bfb563a2b8a0c740da5995d8ed850be

Likewise with GetTemporalCalendarWithISODefault. This reflects that what comes out of these operations is not necessarily a Calendar object, but could be a string identifier of a built-in calendar. It's described as "a value suitable for storing in a Temporal object's [[Calendar]] slot". ToTemporalCalendarWithISODefault is subsumed into ToTemporalCalendarSlotValue. UPSTREAM_COMMIT=5cc322aba186061ee3fb9ff64201f219b74bdab1

The toJSON method should return the value in the internal slot, instead of making an observable call to toString. See: #1808 UPSTREAM_COMMIT=9199c05fdd58ddf45a03211396762e57da4fe6bc

12wrigja · 2023-04-25T16:25:53Z

lib/calendar.ts

    fields: Params['dateFromFields'][0],
    options: NonNullable<Params['dateFromFields'][1]>,
-    calendar: Temporal.Calendar
+    calendar: string


If this is meant to be the calendar's identifier, should the parameter be renamed to indicate that?

I agree, but I'm loath to introduce a deviation from upstream for something as minor as parameter names, esp. because in TS we know the type from the annotation. I'd prefer to save this until after upstream commits have quieted down.

Sorry to be so sensitive about this but I've spent so much time on rebasing recently!

lib/slots.ts

lib/plaindate.ts

lib/zoneddatetime.ts

lib/legacydate.ts

12wrigja · 2023-04-25T17:12:35Z

lib/ecmascript.ts

  return 0;
 }

+// Not abstract operations from the spec


Hm - maybe for ease of organization we should move all the prod polyfill helper fns to a new file so that rebasing doesn't have to get confusing as the upstream file grows?

Maybe for another PR.

I don't expect many new examples like this before Stage 4, so I'm inclined to wait on this. But yeah, probably a good idea to consider move prod-polyfill stuff into a separate file.

12wrigja · 2023-04-25T17:13:09Z

lib/ecmascript.ts

-  const TemporalTimeZone = GetIntrinsic('%Temporal.TimeZone%');
-  return new TemporalTimeZone(ParseTemporalTimeZone(fmt.resolvedOptions().timeZone));
+export function DefaultTimeZone() {
+  return new IntlDateTimeFormat().resolvedOptions().timeZone;


Nit: I wonder if this should somehow use the DateTimeFormat cache from intl.ts.

The time zone can change dynamically, and sadly the only way to learn about the change is to create an expensive DateTimeFormat object. There's a proposal to add a window.ontimezonechange event but it seems stalled. whatwg/html#3047

As we get closer to 1.0, we could think about some optimization settings, e.g. assume that time zone never changes for server apps. Or to cache values for some amount of time that could be adjusted by config when the polyfill is loaded.

Refactor so that ZonedDateTime's [[TimeZone]] internal slot can now store either a string (builtin time zone) or an object (custom time zone). The .timeZone getter is replaced with a .timeZoneId getter that always returns a string, and a .getTimeZone() method that always returns a new object if the slot stored a string. The operations ToTemporalTimeZoneIdentifier and ToTemporalTimeZoneObject convert a [[TimeZone]] slot value to one or the other. See: #1808 UPSTREAM_COMMIT=c56a90c814b4632b87da80be26f4775b0fae1751

When creating Temporal objects with a builtin time zone, we want them to be created with a string in the internal slot, not a TimeZone instance. (For example, when parsing an IANA time zone name out of a string or using UTC as a default.) See: #1808 UPSTREAM_COMMIT=973981239d033fe29bbcaa753bccfcd688e654c3

Previously algorithms such as TimeZoneEquals used ToString to get the identifier of a time zone object. Instead, use ToTemporalTimeZoneIdentifier which observably gets the .id property of a custom time zone instead of getting and calling the .toString property. See: #1808 UPSTREAM_COMMIT=08ae76a9030e7592825317e4da4444f7686a2dd2

This reflects that what comes out of these operations is not necessarily a TimeZone object, but could be a string identifier of a built-in time zone. It's described as "a value suitable for storing in a Temporal.ZonedDateTime's [[TimeZone]] slot". UPSTREAM_COMMIT=5f914c3b456aa9c12cecc9434205f26a5959fe50

The toJSON method should return the value in the internal slot, instead of making an observable call to toString. See: #1808 UPSTREAM_COMMIT=51bb26b9392b90be6a7341a8af2332559639107e

See #2423 for the issue and #2424 for the spec text change, which was already adopted some time ago. I thought the reference code already worked like this, but it does not. Anba pointed this out in tc39/test262#3802 and provided correct test262 tests for the feature. Note that the big-integer library doesn't have a floor division function. We roll our own. If the divisor or dividend is negative (but not both) and there is a remainder, then we subtract 1 from the result (round towards negative infinity instead of 0). UPSTREAM_COMMIT=6e0ad2030170540084a4a400005cc2bc8c791bdb

Previously, "nested" time zone property bags were unwrapped up to one level. That is, this object: { timeZone: { // ...Temporal.TimeZone methods } } would not be considered to implement the TimeZone protocol, but would have its timeZone property used instead, if it were passed to an API that required a TimeZone protocol object. These nested property bags are no longer supported. Discussion: tc39/proposal-temporal#2104 (comment) See: #2104 UPSTREAM_COMMIT=1b097998da0bc4bd2a98aaf3427ecca4d707f7ec

Previously, "nested" calendar property bags were unwrapped up to one level. That is, this object: { calendar: { // ...Temporal.Calendar methods } } would not be considered to implement the Calendar protocol, but would have its calendar property used instead, if it were passed to an API that required a Calendar protocol object. These nested property bags are no longer supported. Discussion: tc39/proposal-temporal#2104 (comment) See: #2104 UPSTREAM_COMMIT=f431988a6eb4b02c056af820aa7f3c0d0d973d22

Checking whether an object implements the TimeZone protocol is now done by means of HasProperty operations for each of the required methods unless the object already has the TimeZone brand. Discussion: tc39/proposal-temporal#2104 (comment) See: #2104 UPSTREAM_COMMIT=9852b94aaeee917e6822013afd89a3cc21294350

Checking whether an object implements the Calendar protocol is now done by means of HasProperty operations for each of the required methods unless the object already has the Calendar brand. Discussion: tc39/proposal-temporal#2104 (comment) See: #2104 UPSTREAM_COMMIT=0ee7c6e2fc0f4b61fb01a4187fef949a3c10c57d

…on.prototype.round If we balance the bigger units before the smaller ones, then we never properly balance the days that are added from balancing units up to day length. Changing the order of operations ensures proper balancing of all units. There is also no more need for the call to MoveRelativeZonedDateTime() in the end of the method, so it's been removed. Closes: #2508 UPSTREAM_COMMIT=36746e5ef8da4688237c195e51c8081e3304ab0f

See PR #2479 about which a consensus was not reached. This change allows Temporal.ZonedDateTime.prototype.toLocaleString to work by overriding the time zone at the time of creating an Intl.DateTimeFormat object and formatting the corresponding Temporal.Instant, but disallows calling any of the Intl.DateTimeFormat methods on a Temporal.ZonedDateTime. NOTE: The reference code does not implement the spec exactly as written. It observably modifies the options before passing them to the real Intl.DateTimeFormat constructor. The behaviour described in the spec is the correct behaviour. UPSTREAM_COMMIT=b8e56c21cefbdb0ea68c3380a9d83c702461e0b9

400 years in the ISO calendar is always exactly 146097 days (400 × 365, plus 97 extra days from leap years.) So, if we have a days value in BalanceISODate that's more than 400 years, we can convert a multiple of 400 years all at once, rather than looping through every year and determining whether it's a leap year. No corresponding spec change needed, as this is not observable from JS. (It's covered by the vague "Find a finite time value t such that YearFromTime(t) is ym, MonthFromTime(t) is mn, and DateFromTime(t) is 1𝔽; but if this is not possible (because some argument is out of range), return NaN" step in MakeDay.) UPSTREAM_COMMIT=9440a1ad4eff137d6acb219d491fc2cbc030b4fb

When working with an epoch nanoseconds value at the start or end of the range, and a UTC offset with the correct sign, it may happen that GetNamedTimeZoneDateTimeParts produces date-time parts that cannot be passed back into GetUTCEpochNanoseconds because they are out of range in UTC. Use the same optimization as in the previous commit: in the absence of changes in UTC offset, 400 years has a constant length. So calculate the epoch nanoseconds in UTC with the remainder of dividing the year by 400, and then add back the number of nanoseconds in the 400 year blocks. UPSTREAM_COMMIT=ad9997d4a860bc2b987bf37c96f4cc37f9d29ccc

We already have a constant BEFORE_FIRST_DST for an exact time before the first UTC offset shift in the IANA time zone database. Calling GetNamedTimeZoneNextTransition on any time before this is equivalent to calling it on BEFORE_FIRST_DST. This speeds up an (admittedly uncommon) operation from many seconds in the most pathological case (nsMinInstant) to a few tens of milliseconds, on my machine. UPSTREAM_COMMIT=19382ba7d080b8bb9ed2f8f730f3429e524c1417

If we find ourselves searching for previous transition times before the first known transition, or next transition times after the end of the Instant range, that means there are none and we have to return null. UPSTREAM_COMMIT=c18ef460cfd02f0c1053779624e51b4f834e0266

…ition There are two time zones that have their DST transitions precomputed until 2087. Requesting the previous transition for an instant far in the future breaks our optimization that assumes zones either have transitions in the next year, or none at all. Special case these two time zones so that they don't take minutes to look up the previous transition for instants long after 2087. UPSTREAM_COMMIT=1b3af0665ae21f5af8966c7457d02e035532111c

UPSTREAM_COMMIT=6a91f6e4775ff1b4d385af33c568ddc0d6f963fb

My initial implementation of ES.CopyDataProperties didn't support symbol keys, which caused two tests to fail.

justingrant

Thanks for the review! A few changes are in fixup commits and are now rebased to main, and I don't think immediate action is needed on the others.

Only one more porting PR remaining to catch up to upstream!

justingrant · 2023-04-25T20:29:22Z

lib/ecmascript.ts

-  const TemporalTimeZone = GetIntrinsic('%Temporal.TimeZone%');
-  return new TemporalTimeZone(ParseTemporalTimeZone(fmt.resolvedOptions().timeZone));
+export function DefaultTimeZone() {
+  return new IntlDateTimeFormat().resolvedOptions().timeZone;


The time zone can change dynamically, and sadly the only way to learn about the change is to create an expensive DateTimeFormat object. There's a proposal to add a window.ontimezonechange event but it seems stalled. whatwg/html#3047

As we get closer to 1.0, we could think about some optimization settings, e.g. assume that time zone never changes for server apps. Or to cache values for some amount of time that could be adjusted by config when the polyfill is loaded.

justingrant · 2023-04-25T20:30:58Z

lib/ecmascript.ts

  return 0;
 }

+// Not abstract operations from the spec


I don't expect many new examples like this before Stage 4, so I'm inclined to wait on this. But yeah, probably a good idea to consider move prod-polyfill stuff into a separate file.

lib/legacydate.ts

lib/zoneddatetime.ts

lib/plaindate.ts

justingrant · 2023-04-26T00:31:49Z

lib/calendar.ts

    fields: Params['dateFromFields'][0],
    options: NonNullable<Params['dateFromFields'][1]>,
-    calendar: Temporal.Calendar
+    calendar: string


I agree, but I'm loath to introduce a deviation from upstream for something as minor as parameter names, esp. because in TS we know the type from the annotation. I'd prefer to save this until after upstream commits have quieted down.

Sorry to be so sensitive about this but I've spent so much time on rebasing recently!

lib/slots.ts

justingrant and others added 2 commits April 25, 2023 01:07

Update test262 before porting more commits

643155f

Normative: Replace Now.timeZone with Now.timeZoneId

9892437

Temporal.Now.timeZone() is replaced by Temporal.Now.timeZoneId() which always returns a time zone identifier string, rather than a TimeZone instance. See: #1808 UPSTREAM_COMMIT=d67db4a7731d13f9e6e2b4be4f6d10732756c06e

justingrant requested review from 12wrigja, Ms2ger and ptomato April 25, 2023 09:12

justingrant force-pushed the port-commits-2023-04-25 branch from 8076f9b to df321af Compare April 25, 2023 10:00

ptomato added 6 commits April 25, 2023 03:23

Normative: Change Calendar.prototype.toJSON not to call toString

70a4e23

The toJSON method should return the value in the internal slot, instead of making an observable call to toString. See: #1808 UPSTREAM_COMMIT=9199c05fdd58ddf45a03211396762e57da4fe6bc

justingrant force-pushed the port-commits-2023-04-25 branch from df321af to 49817ac Compare April 25, 2023 10:23

12wrigja approved these changes Apr 25, 2023

View reviewed changes

ptomato and others added 16 commits April 25, 2023 17:36

Normative: Change TimeZone.prototype.toJSON not to call toString

232fcf6

The toJSON method should return the value in the internal slot, instead of making an observable call to toString. See: #1808 UPSTREAM_COMMIT=51bb26b9392b90be6a7341a8af2332559639107e

ptomato and others added 4 commits April 25, 2023 17:36

Re-enable slow (>15s total) zone transition tests

9f3b380

UPSTREAM_COMMIT=6a91f6e4775ff1b4d385af33c568ddc0d6f963fb

Support symbols in ES.CopyDataProperties

dc312fc

My initial implementation of ES.CopyDataProperties didn't support symbol keys, which caused two tests to fail.

Update expected failures: ES5 & old Node versions

1da82fd

justingrant commented Apr 26, 2023

View reviewed changes

justingrant force-pushed the port-commits-2023-04-25 branch from 49817ac to 1da82fd Compare April 26, 2023 00:40

justingrant merged commit 7ac976d into js-temporal:main Apr 26, 2023

justingrant mentioned this pull request Apr 26, 2023

Cannot call fromEpochMilliseconds in node version 14+ tc39/proposal-temporal#2565

Closed

Port commits from proposal-temporal 2023-04-25 #247

Port commits from proposal-temporal 2023-04-25 #247

Uh oh!

Conversation

justingrant commented Apr 25, 2023

Breaking Changes

Replacing timeZone and calendar properties (tc39/proposal-temporal#2482)

Removed Intl.DateTimeFormat formatting support for Temporal.ZonedDateTime objects (tc39/proposal-temporal#2522)

Uh oh!

12wrigja Apr 25, 2023

Choose a reason for hiding this comment

Uh oh!

justingrant Apr 26, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

12wrigja Apr 25, 2023

Choose a reason for hiding this comment

Uh oh!

justingrant Apr 25, 2023

Choose a reason for hiding this comment

Uh oh!

12wrigja Apr 25, 2023

Choose a reason for hiding this comment

Uh oh!

justingrant Apr 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justingrant left a comment

Choose a reason for hiding this comment

Uh oh!

justingrant Apr 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justingrant Apr 25, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

justingrant Apr 26, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Replacing `timeZone` and `calendar` properties (tc39/proposal-temporal#2482)

Removed `Intl.DateTimeFormat` formatting support for `Temporal.ZonedDateTime` objects (tc39/proposal-temporal#2522)

justingrant Apr 25, 2023 •

edited

Loading

justingrant Apr 25, 2023 •

edited

Loading