Conversation
ptomato
left a comment
There was a problem hiding this comment.
Self-reviewing to point out places where you might particularly have questions/comments and places where the one-document approach would simplify things.
We may want to additionally look at https://tc39.es/ecma262/#sec-date-time-string-format. Most likely this can be simplified with Temporal stuff, or turned into a grammar using the building blocks from Temporal's RFC9557 grammar.
spec.html
Outdated
| In time zone aware implementations, a primary time zone identifier is a Zone name, and a non-primary time zone identifier is a Link name, respectively, in the IANA Time Zone Database except as specifically overridden by AvailableNamedTimeZoneIdentifiers as specified in the ECMA-402 specification. | ||
| Implementations that do not support the entire IANA Time Zone Database are still recommended to use IANA Time Zone Database names as identifiers to represent time zones. | ||
| </p> | ||
| <p>These identifiers are described by the following grammar.</p> |
There was a problem hiding this comment.
This grammar would be deduplicated into Temporal's RFC 9557 grammar if we go the same-document route.
spec.html
Outdated
| UTC offsets that represent offset time zone identifiers, or that are intended for interoperability with ISO 8601, use only hours and minutes and are specified by |UTCOffset[~SubMinutePrecision]|. | ||
| UTC offsets that represent the offset of a named time zone can be more precise, and are specified by |UTCOffset[+SubMinutePrecision]|. | ||
| </p> | ||
| <p>These formats are described by the following grammar.</p> |
There was a problem hiding this comment.
This grammar would also be deduplicated into Temporal's RFC 9557 grammar.
spec.html
Outdated
| </emu-clause> | ||
| </emu-clause> | ||
|
|
||
| <emu-clause id="sec-iso-date-records"> |
There was a problem hiding this comment.
These three sections on ISO Date Records, Time Records, and ISO Date-Time Records would go into a Temporal section if we put Temporal in the same document. But they're needed in ECMA-262 for AOs like GetNamedTimeZoneEpochNanoseconds and GetNamedTimeZoneOffsetNanoseconds.
b7627f5 to
283e9b8
Compare
|
As for the "esmeta yet phrases detection" job there seem to be two main issues:
I'm puzzled about that since both constructions seem to be used elsewhere in the spec. Am I doing something wrong here or do I need to add these manually to a skip list? Once those are resolved I can dig into the remaining items turned up by ESMeta. |
This commit makes changes to ECMA-262 AOs that are needed for Temporal regardless of whether it lands in a separate document.
283e9b8 to
ccbb91e
Compare
This adds the rest of the Temporal proposal, organizing it into the "Numbers and Dates" section. Some of the sections added in the previous commit are moved into Temporal sections. The existing section on "Time Zone Identifiers", used both by Date and Temporal, is moved into its own section under "Numbers and Dates". Most of the proposal is kept in separate temporal/*.emu files, included with `<emu-import>` tags. This is an arbitrary choice and can be done differently if so desired.
ptomato
left a comment
There was a problem hiding this comment.
More self-review with comments on things you may want to look at. I'm planning to join the editor call on Monday 23 Feb, so we can talk through this PR live.
Additional comments not tied to a particular place in the PR:
- We may want to refactor/rewrite 21.4.1.32 Date Time String Format to use the new grammar.
- Probably one of the main things to talk about in the editor call is "how to structure the sections". I've put most everything as subsections of the "Numbers and Dates" section. Maybe it makes sense to split that up into two toplevel sections, "Numbers" and "Dates"? Here's what the second-level outline looks like now:
temporal/abstractops.emu
Outdated
| <emu-clause id="sec-canonicalizeuvalue" type="abstract operation"> | ||
| <h1> | ||
| CanonicalizeUValue ( | ||
| _ukey_: a Unicode locale extension sequence key defined in <a href="https://unicode.org/reports/tr35/#Key_And_Type_Definitions_">Unicode Technical Standard #35 Part 1 Core Section 3.6.1 Key and Type Definitions</a>, | ||
| _uvalue_: a String, | ||
| ): a String | ||
| </h1> | ||
| <dl class="header"> | ||
| <dt>description</dt> | ||
| <dd>The returned String is the canonical and case-regularized form of _uvalue_ as a value of _ukey_.</dd> | ||
| </dl> | ||
| <emu-alg> | ||
| 1. Let _lowerValue_ be the ASCII-lowercase of _uvalue_. | ||
| 1. Let _canonicalized_ be the String value resulting from canonicalizing _lowerValue_ as a value of key _ukey_ per <a href="https://unicode.org/reports/tr35/#processing-localeids">Unicode Technical Standard #35 Part 1 Core, Annex C LocaleId Canonicalization Section 5 Canonicalizing Syntax, Processing LocaleIds</a>. | ||
| 1. NOTE: It is recommended that implementations use the 'u' extension data in <code>common/bcp47</code> provided by the Common Locale Data Repository (available at <a href="https://cldr.unicode.org/">https://cldr.unicode.org/</a>). | ||
| 1. Return _canonicalized_. | ||
| </emu-alg> | ||
| </emu-clause> |
There was a problem hiding this comment.
CanonicalizeUValue is debatable whether it should be included here. It's moved from ECMA-402 into here, but is only used for CanonicalizeCalendar, to show that if the implementation supports multiple calendars, their identifiers should be canonicalized in the way specified in UTS#35.
An alternative would be to override CanonicalizeCalendar in ECMA-402 instead, and provide a skeleton CanonicalizeCalendar here which does something like "If id is some case-insensitive form of "iso8601", return "iso8601", else throw".
There was a problem hiding this comment.
I've included what this would look like, in an additional commit named "(optional) Change approach for specifying CanonicalizeCalendar".
|
Hi! I'm implementing Temporal in engine262. I'd like to know if all changes made here will be synced to the Temporal repo, or if the repo will be stale and all new changes will be made here. |
|
I'm not planning to sync changes back to the Temporal repo, unless someone really wants that. |
|
The rendered spec preview for this PR is available as a single page at https://tc39.es/ecma262/pr/3759 and as multiple pages at https://tc39.es/ecma262/pr/3759/multipage . |
|
Notes from editor call:
|
Split the "Numbers and Dates" section into "Numbers and Mathematical Functions" and "Dates and Temporal".
…rmat 24:00 is not supported in the current (2019) edition of ISO 8601, unlike the 2004 edition which we previously cited in the bibliography. Also update the note about a standard for time zone names not existing; this exists now with RFC 9557.
The desired convention going forward ("Stop Coercing Things") is too
different from the ECMA-402 options conventions, so it doesn't make
sense to share GetOption with ECMA-402.
The reversion of base64 changes can be squashed with "Normative:
Temporal stage 4, part 1"
Minor editorial point, reads better when the operands are in the same order as the AO's arguments.
[[EpochNanoseconds]] should be a mathematical value instead of a BigInt, both as a matter of convention and because implementations are likely to use a different type than their internal BigInt type. Bikeshedding welcome on the type name. I went with "An epoch nanoseconds value", but "an exact time", "an epoch time", etc., are possibilities.
Can be squashed with "Normative: Temporal stage 4, part 2"
…rings Instead of |DateYear| and |DateDay|. Naming the parse node one level down from the goal may make it easier to understand what needs to be implemented.
As discussed during the editors meeting, this commit shows what it'd look like if we changed AvailableCalendars and CanonicalizeCalendar to work more like time zones, where we give the specification assuming that only the minimum is supported. CanonicalizeUValue would then stay in ECMA-402 where it originated, and ECMA-402 would gain an override of CanonicalizeCalendar (it already has one for AvailableCalendars).
|
I've added commits addressing all the things we discussed in Monday's editor call. (To make it easy to see what changed, I'll refrain from squashing or force-pushing until it comes time to merge.) I tried to request another preview but the job failed with a 403 on |
Review suggestion from Michael Dyck.
This was a confusion on my part because in implementations you can optimize out the weeks and days parameters entirely. h/t Jack Works
|
Pushed another commit addressing a comment from @Jack-Works (tc39/proposal-temporal#3290) |
| <emu-clause id="sec-rfc9557grammar-static-semantics-early-errors"> | ||
| <h1>Static Semantics: Early Errors</h1> |
There was a problem hiding this comment.
I think we're missing a syntax error for |TimeZoneIANAName| productions when |TimeZoneIANANameComponent| is "." or "..", which should be added for alignment with RFC 9557.
There was a problem hiding this comment.
I don't think it matters that much, since an annotation like [../../Foobar] will throw a RangeError anyway when given to Instant.from() or ZonedDateTime.from(), because there won't be an IANA time zone with that name.
But note that it would technically be a normative change to make it into a syntax error, because Temporal.PlainDate.from('2026-03-06T15[../../Foobar]') would throw an error instead of ignoring the annotation (as it would with an otherwise syntactically valid annotation such as Temporal.PlainDate.from('2026-03-06T15[Europe/Custom]')).
There was a problem hiding this comment.
I've pushed a commit marked "(optional)" implementing this, if we choose to go ahead with it. But I'd personally recommend against at this point.
There was a problem hiding this comment.
I'd prefer you not include that commit in this PR unless you're planning to ask for consensus for the normative change.
There was a problem hiding this comment.
I did not ask for consensus on it, so I've pushed another commit reverting it.
Review suggestion from Richard.
This reverts commit 4b0645e.
| </dl> | ||
| <emu-alg> | ||
| 1. If _value_ is not an Object, return *false*. | ||
| 1. If _value_ has an [[InitializedTemporalDate]], [[InitializedTemporalDateTime]], [[InitializedTemporalMonthDay]], [[InitializedTemporalTime]], [[InitializedTemporalYearMonth]], or [[InitializedTemporalZonedDateTime]] internal slot, return *false*. |
There was a problem hiding this comment.
As noted at tc39/ecma402#1044 (comment) , it might be worth <dfn>ing something like "Temporal data object" for replacing the various lists like this one.
There was a problem hiding this comment.
@gibson042 How important is this to you? I think I'd rather keep it as is for now, but I don't feel strongly about that.
If we add the definition, I don't love the name "Temporal data object", so maybe we could iterate on that.
There was a problem hiding this comment.
I can live with this as it is now, but would prefer to simplify things with a <dfn>. And I'm very open to alternatives for that suggested term—maybe "Temporal value"?
There was a problem hiding this comment.
One sticking point is that we don't want to include Temporal.Duration, so how to express "Temporal object but not Duration"?
These variables had the wrong types, and needed to be converted from Date Duration Record to Internal Duration Record to make the types match up. No practical change needed for implementations. h/t Tim Chevalier
spec.html
Outdated
| <emu-clause id="sec-tointegerifintegral" type="abstract operation"> | ||
| <h1> | ||
| ToIntegerIfIntegral ( | ||
| _argument_: an ECMAScript language value, | ||
| ): either a normal completion containing an integer or a throw completion | ||
| </h1> | ||
| <dl class="header"> | ||
| <dt>description</dt> | ||
| <dd>It converts _argument_ to an integer representing its Number value, or throws a *RangeError* when that value is not <emu-xref href="#integral-number">integral</emu-xref>.</dd> | ||
| </dl> | ||
| <emu-alg> | ||
| 1. Let _number_ be ? ToNumber(_argument_). | ||
| 1. If _number_ is not an integral Number, throw a *RangeError* exception. | ||
| 1. Return ℝ(_number_). | ||
| </emu-alg> | ||
| </emu-clause> | ||
|
|
||
| <emu-clause id="sec-tointegerwithtruncation" type="abstract operation"> | ||
| <h1> | ||
| ToIntegerWithTruncation ( | ||
| _argument_: an ECMAScript language value, | ||
| ): either a normal completion containing an integer or a throw completion | ||
| </h1> | ||
| <dl class="header"> | ||
| <dt>description</dt> | ||
| <dd>It converts _argument_ to an integer representing its Number value with fractional part truncated, or throws a *RangeError* when that value is not finite.</dd> | ||
| </dl> | ||
| <emu-alg> | ||
| 1. Let _number_ be ? ToNumber(_argument_). | ||
| 1. If _number_ is one of *NaN*, *+∞*<sub>𝔽</sub>, or *-∞*<sub>𝔽</sub>, throw a *RangeError* exception. | ||
| 1. Return truncate(ℝ(_number_)). | ||
| </emu-alg> | ||
| </emu-clause> | ||
|
|
||
| <emu-clause id="sec-topositiveintegerwithtruncation" type="abstract operation"> | ||
| <h1> | ||
| ToPositiveIntegerWithTruncation ( | ||
| _argument_: an ECMAScript language value, | ||
| ): either a normal completion containing a positive integer or a throw completion | ||
| </h1> | ||
| <dl class="header"> | ||
| <dt>description</dt> | ||
| <dd>It converts _argument_ to an integer representing its Number value with fractional part truncated, or throws a *RangeError* when that value is not finite or not positive.</dd> | ||
| </dl> | ||
| <emu-alg> | ||
| 1. Let _integer_ be ? ToIntegerWithTruncation(_argument_). | ||
| 1. If _integer_ ≤ 0, throw a *RangeError* exception. | ||
| 1. Return _integer_. | ||
| </emu-alg> | ||
| </emu-clause> |
There was a problem hiding this comment.
An observation from proposal-amount: these three proposed operations could instead be implemented as a single new operation.
| <emu-clause id="sec-tointegerifintegral" type="abstract operation"> | |
| <h1> | |
| ToIntegerIfIntegral ( | |
| _argument_: an ECMAScript language value, | |
| ): either a normal completion containing an integer or a throw completion | |
| </h1> | |
| <dl class="header"> | |
| <dt>description</dt> | |
| <dd>It converts _argument_ to an integer representing its Number value, or throws a *RangeError* when that value is not <emu-xref href="#integral-number">integral</emu-xref>.</dd> | |
| </dl> | |
| <emu-alg> | |
| 1. Let _number_ be ? ToNumber(_argument_). | |
| 1. If _number_ is not an integral Number, throw a *RangeError* exception. | |
| 1. Return ℝ(_number_). | |
| </emu-alg> | |
| </emu-clause> | |
| <emu-clause id="sec-tointegerwithtruncation" type="abstract operation"> | |
| <h1> | |
| ToIntegerWithTruncation ( | |
| _argument_: an ECMAScript language value, | |
| ): either a normal completion containing an integer or a throw completion | |
| </h1> | |
| <dl class="header"> | |
| <dt>description</dt> | |
| <dd>It converts _argument_ to an integer representing its Number value with fractional part truncated, or throws a *RangeError* when that value is not finite.</dd> | |
| </dl> | |
| <emu-alg> | |
| 1. Let _number_ be ? ToNumber(_argument_). | |
| 1. If _number_ is one of *NaN*, *+∞*<sub>𝔽</sub>, or *-∞*<sub>𝔽</sub>, throw a *RangeError* exception. | |
| 1. Return truncate(ℝ(_number_)). | |
| </emu-alg> | |
| </emu-clause> | |
| <emu-clause id="sec-topositiveintegerwithtruncation" type="abstract operation"> | |
| <h1> | |
| ToPositiveIntegerWithTruncation ( | |
| _argument_: an ECMAScript language value, | |
| ): either a normal completion containing a positive integer or a throw completion | |
| </h1> | |
| <dl class="header"> | |
| <dt>description</dt> | |
| <dd>It converts _argument_ to an integer representing its Number value with fractional part truncated, or throws a *RangeError* when that value is not finite or not positive.</dd> | |
| </dl> | |
| <emu-alg> | |
| 1. Let _integer_ be ? ToIntegerWithTruncation(_argument_). | |
| 1. If _integer_ ≤ 0, throw a *RangeError* exception. | |
| 1. Return _integer_. | |
| </emu-alg> | |
| </emu-clause> | |
| <emu-clause id="sec-snaptointeger" type="abstract operation"> | |
| <h1> | |
| SnapToInteger ( | |
| _argument_: an ECMAScript language value, | |
| _mode_: ~strict~ or ~truncate-strict~, | |
| optional _minimum_: an integer, | |
| optional _maximum_: an integer, | |
| ): either a normal completion containing an integer or a throw completion | |
| </h1> | |
| <dl class="header"> | |
| <dt>description</dt> | |
| <dd>It converts _argument_ to an integer representing its Number value, rejecting a non-finite value and handling a finite non-<emu-xref href="#integral-number">integral</emu-xref> value by either rejecting or truncating it, and optionally imposes inclusive constraining bounds.</dd> | |
| </dl> | |
| <emu-alg> | |
| 1. Let _number_ be ? ToNumber(_argument_). | |
| 1. If _number_ is one of *NaN*, *+∞*<sub>𝔽</sub>, or *-∞*<sub>𝔽</sub>, throw a *RangeError* exception. | |
| 1. Let _mv_ be ℝ(_number_). | |
| 1. If _mode_ is ~truncate-strict~, set _mv_ to truncate(_mv_). | |
| 1. Else if _mv_ is not an integer, throw a *RangeError* exception. | |
| 1. If _minimum_ is present and _mv_ < _minimum_, throw a *RangeError* exception. | |
| 1. If _maximum_ is present and _mv_ > _maximum_, throw a *RangeError* exception. | |
| 1. Return _mv_. | |
| </emu-alg> | |
| </emu-clause> |
1. If _Conversion_ is ~to-integer-with-truncation~, then
- 1. Set _value_ to ? ToIntegerWithTruncation(_value_).
+ 1. Set _value_ to ? SnapToInteger(_value_, ~truncate-strict~).
1. Set _value_ to 𝔽(_value_).
1. Else if _Conversion_ is ~to-positive-integer-with-truncation~, then
- 1. Set _value_ to ? ToPositiveIntegerWithTruncation(_value_).
+ 1. Set _value_ to ? SnapToInteger(_value_, ~truncate-strict~, 1).
1. Set _value_ to 𝔽(_value_).- 1. If _years_ is *undefined*, let _y_ be 0; else let _y_ be ? ToIntegerIfIntegral(_years_).
+ 1. If _years_ is *undefined*, let _y_ be 0; else let _y_ be ? SnapToInteger(_y_, ~strict~).There was a problem hiding this comment.
I would like to avoid scope creep on this PR. That said, this wasn't too difficult, I'll add a commit onto the stack marked "(optional)". If it turns out this generates a lot of bikeshedding, then I'd prefer to revert it back to separate operations and save SnapToInteger for a future change.
| 1. Let _offsetNs_ be _parseResult_.[[OffsetMinutes]] × (60 × 10<sup>9</sup>). | ||
| 1. Else, | ||
| 1. Let _possibleInstants_ be GetNamedTimeZoneEpochNanoseconds(_systemTimeZoneIdentifier_, ℝ(YearFromTime(_t_)), ℝ(MonthFromTime(_t_)) + 1, ℝ(DateFromTime(_t_)), ℝ(HourFromTime(_t_)), ℝ(MinFromTime(_t_)), ℝ(SecFromTime(_t_)), ℝ(msFromTime(_t_)), 0, 0). | ||
| 1. Let _isoDateTime_ be TimeValueToISODateTimeRecord(_t_). |
There was a problem hiding this comment.
t is a Number, but TimeValueToISODateTimeRecord requires a finite time value (a time value is NaN or finite int). You did not check if it is an int.
There was a problem hiding this comment.
This is a pre-existing problem. I believe, based on all the current callers of UTC, the type of t should be "integral Number or NaN", but I'm hesitant to change that in this PR without a go-ahead from the editors. It would involve changing the types of MakeDate etc.
See also #3464, not the same but related.
| 1. Let _minute_ be ToZeroPaddedDecimalString(ℝ(MinFromTime(_tv_)), 2). | ||
| 1. Let _second_ be ToZeroPaddedDecimalString(ℝ(SecFromTime(_tv_)), 2). | ||
| 1. Return the string-concatenation of _hour_, *":"*, _minute_, *":"*, _second_, the code unit 0x0020 (SPACE), and *"GMT"*. | ||
| 1. Let _timeString_ be FormatTimeString(ℝ(HourFromTime(_tv_)), ℝ(MinFromTime(_tv_)), ℝ(SecFromTime(_tv_)), 0, 0). |
There was a problem hiding this comment.
type mismatch: HourFromTime, MinFromTime, SecFromTime both require a finite time value (int), but t is finite number
There was a problem hiding this comment.
Same response as above.
This should be an integer, not a floating-point value.
| 1. Set _milliseconds_ to floor(_microseconds_ / 1000). | ||
| 1. Set _microseconds_ to _microseconds_ modulo 1000. | ||
| 1. Else if _largestUnit_ is ~microsecond~, then | ||
| 1. Set _microseconds_ to floor(_nanoseconds_ / 1000). |
There was a problem hiding this comment.
| 1. Set _microseconds_ to floor(_nanoseconds_ / 1000). | |
| 1. Set _microseconds_ to R(F(floor(_nanoseconds_ / 1000))). |
This precision loss is implemented in TemporalDurationFromInternal in https://tc39.es/proposal-temporal/docs/playground.js, it calls FMAPowerOf10 to do the precision loss.
Without this precision loss, this test will fail:
built-ins/Temporal/Duration/prototype/round/float64-representable-integer.js
Test requires: Temporal.Duration <PT18014398509.48198S>
Current spec: Temporal.Duration <PT18014398509.481981S>
There was a problem hiding this comment.
The R(F(...)) happens in CreateTemporalDuration, so it doesn't need to be repeated here.
There was a problem hiding this comment.
| 1. Let _nanosecondsString_ be the substring of _fraction_ from 0 to 9. |
If "substring" here is in the sense of JavaScript String.prototype.substring.
Failed case:
Temporal.Instant.from("1970-01-01T00:19:32.37+00:19:32.37")parsed as Temporal.Instant <1969-12-31T23:59:59.67Z> if use 1 to 10, correctly as Temporal.Instant <1970-01-01T00:00:00Z> use 0 to 9.
There was a problem hiding this comment.
context: ParseDateTimeUTCOffset step 16.c
There was a problem hiding this comment.
In Temporal.Instant.from("1970-01-01T00:19:32.37+00:19:32.37"), ParseDateTimeUTCOffset is called with offsetString "+00:19:32.37", which it parses using |UTCOffset[+SubMinutePrecision]| and gets back a parseResult Parse Node containing a |TemporalDecimalFraction| whose source text is ".37". Step 16.b then sets fraction to the string-concatenation of that string and "000000000" (i.e., ".37000000000"), 16.c sets nanosecondsString to the substring of fraction from 1 to 10 (i.e., "370000000"), and 16.d sets nanoseconds to that number (i.e., 370_000_000), which is in fact the count of nanoseconds indicated by the ".37" in "+00:19:32.37". So what exactly is the problem?
There was a problem hiding this comment.
Thanks for the clarification, because my parse node does not contain the . dot, so I need to use 0, 9
There was a problem hiding this comment.
However, there does seem to be a problem in ToTemporalInstant for Temporal.Instant.from("1970-01-01T00+00:19:32.37")—step 3 sets parsed to ParseISODateTime(item, « |TemporalInstantString| »), and ParseISODateTime step 7 initializes minute, second, and fSeconds from the first |MinuteSecond|, |TimeSecond|, and the first |TemporalDecimalFraction| from a parseResult Parse Node resulting from parsing "1970-01-01T00+00:19:32.37" as a |TemporalInstantString|. But |MinuteSecond| and |TemporalDecimalFraction| are optional in |Time| ::: |TimeSpec| (and in fact absent in this |TimeSpec| ::: |Hour| case) and in |DateTimeUTCOffset| ::: |UTCOffset| via |HourSubcomponents| (and present in this |HourSubcomponents| case), resulting in «the first |MinuteSecond|» and «the first |TemporalDecimalFraction|» coming from inside |DateTimeUTCOffset| rather than from inside |Time|, which AFAIK is unintentional and incorrect.
Interestingly, initializing second from |TimeSecond| escapes this problem because |DateTimeUTCOffset| doesn't contain |TimeSecond|. The fix could duplicate that for offset minute and fractional seconds, but that seems brittle. I think instead ParseISODateTime step 7 should take care to specifically exclude Parse Nodes that are part of an offset rather than a base date/time/datetime (noting that this affects all use of ParseISODateTime with a nonterminal that includes |DateTimeUTCOffset|, even as in ToTemporalTime).
Stage 4 PR for Temporal.
Draft 2026-02-16:
This currently includes only the changes to ECMA-262 that would need to be made if putting Temporal in a separate document. It does not yet include the Temporal Object itself. I've posted this for early feedback since I expect these parts will be what attracts the most review comments anyway. I'll update this later on to include the rest of the proposal.2026-02-20: This now includes the rest of the Temporal proposal.