Normative: Read date-time options only once when creating DateTimeFormat objects #709

anba · 2022-09-20T14:27:46Z

The first two commits are purely editorial and only move some things around, so it's easier to review the third commit, which contains the actual change.

Fixes #237
Fixes #706

spec/datetimeformat.html

sffc · 2022-10-06T19:51:59Z

TG2 discussion: https://github.com/tc39/ecma402/blob/master/meetings/notes-2022-10-06.md#normative-read-date-time-options-only-once-when-creating-datetimeformat-objects

Conclusion: Review the Test262 impact. Further discussion is not necessary in this group unless red flags show up. Approved with that condition.

ryzokuken · 2022-11-28T17:04:40Z

@gibson042 any updates?

Ms2ger · 2023-01-11T09:14:29Z

Please also note the changes going in a similar direction in Temporal. (I don't have the time right now to review if any changes would be helpful here or there.)

…ateTimeFormat objects" Update tests for PR tc39/ecma402#709.

anba · 2023-01-12T13:26:41Z

Conclusion: Review the Test262 impact.

Test262 PR created at tc39/test262#3768.

spec/datetimeformat.html

justingrant · 2023-01-12T16:01:32Z

Please also note the changes going in a similar direction in Temporal. (I don't have the time right now to review if any changes would be helpful here or there.)

I filed tc39/proposal-temporal#2473 to adapt to this change on the Temporal side.

Ref tc39/ecma402#709 (comment)

@param

Ref tc39/ecma402#709 (comment) Passes relevant tests after minor modification: ```sh $ npm run test:test262 -- --extended-tests --run-slow-tests $( find $( find test/test262/test262/ -type d -a '(' -iname '*intl*' -o -iname '*402*' ')' -prune | \ tee /dev/stderr \ ) -iname '*date*' -prune | \ grep -viE 'temporal|\bdate\b|\bsupportedValuesOf\b|\bDisplayName' ) test/test262/test262/test/staging/Intl402 test/test262/test262/test/intl402 test/test262/test262/implementation-contributed/v8/intl test/test262/test262/implementation-contributed/v8/test262/local-tests/test/intl402 > @engine262/engine262@0.0.1 test:test262 > node --enable-source-maps test/test262/test262.js --extended-tests --run-slow-tests test/test262/test262/test/intl402/DateTimeFormat test/test262/test262/test/intl402/Intl/DateTimeFormat test/test262/test262/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat engine262 Test Runner Detected 4 CPUs Not running on CI [00:04|: 342|+ 322|- 0|» 20] (68.00/s) $ pushd test/test262/test262/ && git diff; popd diff --git i/harness/testIntl.js w/harness/testIntl.js index 286ccd129e..f674055e2c 100644 --- i/harness/testIntl.js +++ w/harness/testIntl.js @@ -42,7 +42,8 @@ defines: * @param {Function} Constructor the constructor object to test with. */ function testWithIntlConstructors(f) { - var constructors = ["Collator", "NumberFormat", "DateTimeFormat"]; + // engine262 does not yet implement Collator or NumberFormat. + var constructors = [/*"Collator", "NumberFormat",*/ "DateTimeFormat"]; // Optionally supported Intl constructors. // NB: Intl.Locale isn't an Intl service constructor! diff --git i/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat/12.1.1_1.js w/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat/12.1.1_1.js index 64845e74a9..bee9c5cf53 100644 --- i/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat/12.1.1_1.js +++ w/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat/12.1.1_1.js @@ -11,7 +11,8 @@ includes: [testIntl.js] testWithIntlConstructors(function (Constructor) { var obj, newObj; - if (Constructor === Intl.DateTimeFormat) { + // engine262 does not implement ChainDateTimeFormat (which is normative-optional). + if (false && Constructor === Intl.DateTimeFormat) { obj = new Constructor(); newObj = Intl.DateTimeFormat.call(obj); if (obj !== newObj) { diff --git i/test/intl402/DateTimeFormat/date-time-options.js w/test/intl402/DateTimeFormat/date-time-options.js index 7d9ef93d20..8c8502fc3b 100644 --- i/test/intl402/DateTimeFormat/date-time-options.js +++ w/test/intl402/DateTimeFormat/date-time-options.js @@ -36,6 +36,8 @@ function testWithDateTimeFormat(options, expected) { } function testWithToLocale(f, options, expected) { + // engine262 does not yet implement Intl-integrated Date.prototype.toLocaleString. + return; // expected can be either one subset or an array of possible subsets if (expected.length === undefined) { expected = [expected]; ```

@param

Ref tc39/ecma402#709 (comment) Passes relevant tests after minor modification: ```sh $ npm run test:test262 -- --extended-tests --run-slow-tests $( find $( find test/test262/test262/ -type d -a '(' -iname '*intl*' -o -iname '*402*' ')' -prune | \ tee /dev/stderr \ ) -iname '*date*' -prune | \ grep -viE 'temporal|\bdate\b|\bsupportedValuesOf\b|\bDisplayName' ) test/test262/test262/test/staging/Intl402 test/test262/test262/test/intl402 test/test262/test262/implementation-contributed/v8/intl test/test262/test262/implementation-contributed/v8/test262/local-tests/test/intl402 > @engine262/engine262@0.0.1 test:test262 > node --enable-source-maps test/test262/test262.js --extended-tests --run-slow-tests test/test262/test262/test/intl402/DateTimeFormat test/test262/test262/test/intl402/Intl/DateTimeFormat test/test262/test262/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat engine262 Test Runner Detected 4 CPUs Not running on CI [00:04|: 342|+ 322|- 0|» 20] (68.00/s) $ pushd test/test262/test262/ && git diff; popd diff --git i/harness/testIntl.js w/harness/testIntl.js index 286ccd129e..f674055e2c 100644 --- i/harness/testIntl.js +++ w/harness/testIntl.js @@ -42,7 +42,8 @@ defines: * @param {Function} Constructor the constructor object to test with. */ function testWithIntlConstructors(f) { - var constructors = ["Collator", "NumberFormat", "DateTimeFormat"]; + // engine262 does not yet implement Collator or NumberFormat. + var constructors = [/*"Collator", "NumberFormat",*/ "DateTimeFormat"]; // Optionally supported Intl constructors. // NB: Intl.Locale isn't an Intl service constructor! diff --git i/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat/12.1.1_1.js w/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat/12.1.1_1.js index 64845e74a9..bee9c5cf53 100644 --- i/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat/12.1.1_1.js +++ w/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat/12.1.1_1.js @@ -11,7 +11,8 @@ includes: [testIntl.js] testWithIntlConstructors(function (Constructor) { var obj, newObj; - if (Constructor === Intl.DateTimeFormat) { + // engine262 does not implement ChainDateTimeFormat (which is normative-optional). + if (false && Constructor === Intl.DateTimeFormat) { obj = new Constructor(); newObj = Intl.DateTimeFormat.call(obj); if (obj !== newObj) { diff --git i/test/intl402/DateTimeFormat/date-time-options.js w/test/intl402/DateTimeFormat/date-time-options.js index 7d9ef93d20..8c8502fc3b 100644 --- i/test/intl402/DateTimeFormat/date-time-options.js +++ w/test/intl402/DateTimeFormat/date-time-options.js @@ -36,6 +36,8 @@ function testWithDateTimeFormat(options, expected) { } function testWithToLocale(f, options, expected) { + // engine262 does not yet implement Intl-integrated Date.prototype.toLocaleString. + return; // expected can be either one subset or an array of possible subsets if (expected.length === undefined) { expected = [expected]; ```

Implements tc39/ecma402#709 Introduced test failures: ```sh $ npm run test:test262 -- --extended-tests --run-slow-tests $(find $(find test/test262/test262/ -type d -a '(' -iname '*intl*' -o -iname '*402*' ')' -prune | tee /dev/stderr) -iname '*date*' -prune | grep -viE 'temporal|\bdate\b|\bsupportedValuesOf\b|\bDisplayName') 2>&1 | grep -vE '^[[:space:]]+ at ' | tee /dev/stderr | awk '/^FAILURE!/ { files[$2]++ } END { printf "\n\n# Files\n" > "/dev/stderr"; for(f in files) print f; }' | sort -u $ npm run test:test262 -- --extended-tests --run-slow-tests $( find $( find test/test262/test262/ -type d -a '(' -iname '*intl*' -o -iname '*402*' ')' -prune | \ tee /dev/stderr \ ) -iname '*date*' -prune | \ grep -viE 'temporal|\bdate\b|\bsupportedValuesOf\b|\bDisplayName' ) 2>&1 | \ grep -vE '^[[:space:]]+ at ' | \ tee /dev/stderr | \ awk '/^FAILURE/ { a[$2]++ } END { print "\n\n# Files" > "/dev/stderr"; for(f in a) print f }' | \ sort -u test/test262/test262/test/staging/Intl402 test/test262/test262/test/intl402 test/test262/test262/implementation-contributed/v8/intl test/test262/test262/implementation-contributed/v8/test262/local-tests/test/intl402 > @engine262/engine262@0.0.1 test:test262 > node --enable-source-maps test/test262/test262.js --extended-tests --run-slow-tests test/test262/test262/test/intl402/DateTimeFormat test/test262/test262/test/intl402/Intl/DateTimeFormat test/test262/test262/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat engine262 Test Runner Detected 4 CPUs Not running on CI [00:00|: 342|+ 0|- 0|» 20] (0.00/s) FAILURE! intl402/DateTimeFormat/constructor-options-order-dayPeriod.js Checks the order of getting options of 'dayPeriod' for the DateTimeFormat constructor. (Strict Mode) Error: Expected [day, dayPeriod, hour] and [day, dayPeriod, hour, day, dayPeriod, hour] to have the same contents. FAILURE! intl402/DateTimeFormat/constructor-options-order-dayPeriod.js Checks the order of getting options of 'dayPeriod' for the DateTimeFormat constructor. Error: Expected [day, dayPeriod, hour] and [day, dayPeriod, hour, day, dayPeriod, hour] to have the same contents. FAILURE! intl402/DateTimeFormat/constructor-options-order-timedate-style.js Checks the order of getting options for the DateTimeFormat constructor. Error: Expected [localeMatcher, hour12, hourCycle, timeZone, weekday, era, year, month, day, hour, minute, second, timeZoneName, formatMatcher, dateStyle, timeStyle] and [weekday, year, month, day, hour, minute, second, dateStyle, timeStyle, localeMatcher, hour12, hourCycle, timeZone, weekday, era, year, month, day, hour, minute, second, timeZoneName, formatMatcher, dateStyle, timeStyle] to have the same contents. FAILURE! intl402/DateTimeFormat/constructor-options-order-fractionalSecondDigits.js Checks the order of getting options of 'fractionalSecondDigits' for the DateTimeFormat constructor. (Strict Mode) Error: Expected [localeMatcher, second, fractionalSecondDigits, timeZoneName, formatMatcher] and [second, fractionalSecondDigits, localeMatcher, second, fractionalSecondDigits, timeZoneName, formatMatcher] to have the same contents. FAILURE! intl402/DateTimeFormat/constructor-options-order.js Checks the order of getting options for the DateTimeFormat constructor. (Strict Mode) Error: Expected [localeMatcher, hour12, hourCycle, timeZone, weekday, era, year, month, day, hour, minute, second, timeZoneName, formatMatcher] and [weekday, year, month, day, hour, minute, second, localeMatcher, hour12, hourCycle, timeZone, weekday, era, year, month, day, hour, minute, second, timeZoneName, formatMatcher] to have the same contents. FAILURE! intl402/DateTimeFormat/constructor-options-order.js Checks the order of getting options for the DateTimeFormat constructor. Error: Expected [localeMatcher, hour12, hourCycle, timeZone, weekday, era, year, month, day, hour, minute, second, timeZoneName, formatMatcher] and [weekday, year, month, day, hour, minute, second, localeMatcher, hour12, hourCycle, timeZone, weekday, era, year, month, day, hour, minute, second, timeZoneName, formatMatcher] to have the same contents. FAILURE! intl402/DateTimeFormat/constructor-options-order-fractionalSecondDigits.js Checks the order of getting options of 'fractionalSecondDigits' for the DateTimeFormat constructor. Error: Expected [localeMatcher, second, fractionalSecondDigits, timeZoneName, formatMatcher] and [second, fractionalSecondDigits, localeMatcher, second, fractionalSecondDigits, timeZoneName, formatMatcher] to have the same contents. FAILURE! intl402/DateTimeFormat/constructor-options-order-timedate-style.js Checks the order of getting options for the DateTimeFormat constructor. (Strict Mode) Error: Expected [localeMatcher, hour12, hourCycle, timeZone, weekday, era, year, month, day, hour, minute, second, timeZoneName, formatMatcher, dateStyle, timeStyle] and [weekday, year, month, day, hour, minute, second, dateStyle, timeStyle, localeMatcher, hour12, hourCycle, timeZone, weekday, era, year, month, day, hour, minute, second, timeZoneName, formatMatcher, dateStyle, timeStyle] to have the same contents. [00:04|: 342|+ 314|- 8|» 20] (70.00/s) intl402/DateTimeFormat/constructor-options-order-dayPeriod.js intl402/DateTimeFormat/constructor-options-order-fractionalSecondDigits.js intl402/DateTimeFormat/constructor-options-order-timedate-style.js intl402/DateTimeFormat/constructor-options-order.js ```

gibson042 · 2023-01-14T20:22:02Z

I confirmed with engine262 that the test changes at tc39/test262#3768 are the correct updates for this PR: engine262/engine262@3248ccc...gibson042:ecma402-gh-709-initializedatetimeformat

And since test changes are required, it's worth looking critically at the new read order proposed here [spoiler—I think slight modification is warranted]:

localeMatcher [affects locale resolution, common to all service constructors]
calendar [affects locale resolution]
numberingSystem [affects locale resolution]
hour12 [affects locale resolution]
hourCycle [affects locale resolution]
timeZone [e.g., "America/New_York"]
component-specific configuration
1. weekday
2. era
3. year
4. month
5. day
6. dayPeriod
7. hour
8. minute
9. second
10. fractionalSecondDigits
11. timeZoneName
formatMatcher ["basic" vs. "best fit" output pattern selection hint]
dateStyle [date-specific output pattern override]
timeStyle [time-specific output pattern override]

Cross-facility comparison:

DisplayNames reads localeMatcher, style ["narrow", "short", "long"], type ["language", "region", etc.], and then narrow configuration.
ListFormat reads localeMatcher, type ["conjunction", "disjunction", "unit"], style ["long", "short", "narrow"].
NumberFormat reads localeMatcher, numberingSystem, style ["decimal", "currency", "unit", etc.—analogous to type in other facilities], style detail fields, and then a hodgepodge of other configuration (notation, compactDisplay ["short", "long"—somewhat similar to style in other facilities], comma and sign control, etc.).
RelativeTimeFormat reads localeMatcher, numberingSystem, style ["long", "short", "narrow"], and then configuration detail.

Considering the above, I think it does make sense to read calendar and numberingSystem where they are. formatMatcher and {date,time}Style form a style/type-like group that should probably precede reading component-specific fields, better aligning with DisplayNames and NumberFormat and RelativeTimeFormat. That leaves timeZone, which acts as input data augmentation and doesn't really have a great analogue in other facilities—I think it can make sense either immediately before or immediately after the aforementioned style group. I'm inclined to say "before", making the overall pattern "locale" then "context" then "style/type" then "details".

Concretely, I propose the narrow change here of moving the formatMatcher/dateStyle/timeStyle block immediately before the components-table loop and also reordering it to dateStyle then timeStyle then formatMatcher (since formatMatcher is subordinate to the other fields, only having an effect when they are both undefined).

EDIT: But I'm also fine with not reordering, since it may not survive #747 anyway.

@param

Ref tc39/ecma402#709 (comment) Passes relevant tests after minor modification: ```sh $ npm run test:test262 -- --extended-tests --run-slow-tests $( find $( find test/test262/test262/ -type d -a '(' -iname '*intl*' -o -iname '*402*' ')' -prune | \ tee /dev/stderr \ ) -iname '*date*' -prune | \ grep -viE 'temporal|\bdate\b|\bsupportedValuesOf\b|\bDisplayName' ) test/test262/test262/test/staging/Intl402 test/test262/test262/test/intl402 test/test262/test262/implementation-contributed/v8/intl test/test262/test262/implementation-contributed/v8/test262/local-tests/test/intl402 > @engine262/engine262@0.0.1 test:test262 > node --enable-source-maps test/test262/test262.js --extended-tests --run-slow-tests test/test262/test262/test/intl402/DateTimeFormat test/test262/test262/test/intl402/Intl/DateTimeFormat test/test262/test262/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat engine262 Test Runner Detected 4 CPUs Not running on CI [00:04|: 342|+ 322|- 0|» 20] (68.00/s) $ pushd test/test262/test262/ && git diff; popd diff --git i/harness/testIntl.js w/harness/testIntl.js index 286ccd129e..f674055e2c 100644 --- i/harness/testIntl.js +++ w/harness/testIntl.js @@ -42,7 +42,8 @@ defines: * @param {Function} Constructor the constructor object to test with. */ function testWithIntlConstructors(f) { - var constructors = ["Collator", "NumberFormat", "DateTimeFormat"]; + // engine262 does not yet implement Collator or NumberFormat. + var constructors = [/*"Collator", "NumberFormat",*/ "DateTimeFormat"]; // Optionally supported Intl constructors. // NB: Intl.Locale isn't an Intl service constructor! diff --git i/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat/12.1.1_1.js w/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat/12.1.1_1.js index 64845e74a9..bee9c5cf53 100644 --- i/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat/12.1.1_1.js +++ w/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat/12.1.1_1.js @@ -11,7 +11,8 @@ includes: [testIntl.js] testWithIntlConstructors(function (Constructor) { var obj, newObj; - if (Constructor === Intl.DateTimeFormat) { + // engine262 does not implement ChainDateTimeFormat (which is normative-optional). + if (false && Constructor === Intl.DateTimeFormat) { obj = new Constructor(); newObj = Intl.DateTimeFormat.call(obj); if (obj !== newObj) { diff --git i/test/intl402/DateTimeFormat/date-time-options.js w/test/intl402/DateTimeFormat/date-time-options.js index 7d9ef93d20..8c8502fc3b 100644 --- i/test/intl402/DateTimeFormat/date-time-options.js +++ w/test/intl402/DateTimeFormat/date-time-options.js @@ -36,6 +36,8 @@ function testWithDateTimeFormat(options, expected) { } function testWithToLocale(f, options, expected) { + // engine262 does not yet implement Intl-integrated Date.prototype.toLocaleString. + return; // expected can be either one subset or an array of possible subsets if (expected.length === undefined) { expected = [expected]; ```

Implements tc39/ecma402#709 Introduced test failures: ```sh $ npm run test:test262 -- --extended-tests --run-slow-tests $( find $( find test/test262/test262/ -type d -a '(' -iname '*intl*' -o -iname '*402*' ')' -prune | \ tee /dev/stderr \ ) -iname '*date*' -prune | \ grep -viE 'temporal|\bdate\b|\bsupportedValuesOf\b|\bDisplayName' ) 2>&1 | \ grep -vE '^[[:space:]]+ at ' | \ tee /dev/stderr | \ awk '/^FAILURE/ { a[$2]++ } END { print "\n\nFILES" > "/dev/stderr"; for(f in a) print f }' | \ sort -u test/test262/test262/test/staging/Intl402 test/test262/test262/test/intl402 test/test262/test262/implementation-contributed/v8/intl test/test262/test262/implementation-contributed/v8/test262/local-tests/test/intl402 > @engine262/engine262@0.0.1 test:test262 > node --enable-source-maps test/test262/test262.js --extended-tests --run-slow-tests test/test262/test262/test/intl402/DateTimeFormat test/test262/test262/test/intl402/Intl/DateTimeFormat test/test262/test262/implementation-contributed/v8/test262/local-tests/test/intl402/DateTimeFormat engine262 Test Runner Detected 4 CPUs Not running on CI [00:00|: 342|+ 0|- 0|» 20] (0.00/s) FAILURE! intl402/DateTimeFormat/constructor-options-order-dayPeriod.js Checks the order of getting options of 'dayPeriod' for the DateTimeFormat constructor. (Strict Mode) Error: Expected [day, dayPeriod, hour] and [day, dayPeriod, hour, day, dayPeriod, hour] to have the same contents. FAILURE! intl402/DateTimeFormat/constructor-options-order-dayPeriod.js Checks the order of getting options of 'dayPeriod' for the DateTimeFormat constructor. Error: Expected [day, dayPeriod, hour] and [day, dayPeriod, hour, day, dayPeriod, hour] to have the same contents. FAILURE! intl402/DateTimeFormat/constructor-options-order-timedate-style.js Checks the order of getting options for the DateTimeFormat constructor. Error: Expected [localeMatcher, hour12, hourCycle, timeZone, weekday, era, year, month, day, hour, minute, second, timeZoneName, formatMatcher, dateStyle, timeStyle] and [weekday, year, month, day, hour, minute, second, dateStyle, timeStyle, localeMatcher, hour12, hourCycle, timeZone, weekday, era, year, month, day, hour, minute, second, timeZoneName, formatMatcher, dateStyle, timeStyle] to have the same contents. FAILURE! intl402/DateTimeFormat/constructor-options-order-fractionalSecondDigits.js Checks the order of getting options of 'fractionalSecondDigits' for the DateTimeFormat constructor. (Strict Mode) Error: Expected [localeMatcher, second, fractionalSecondDigits, timeZoneName, formatMatcher] and [second, fractionalSecondDigits, localeMatcher, second, fractionalSecondDigits, timeZoneName, formatMatcher] to have the same contents. FAILURE! intl402/DateTimeFormat/constructor-options-order.js Checks the order of getting options for the DateTimeFormat constructor. (Strict Mode) Error: Expected [localeMatcher, hour12, hourCycle, timeZone, weekday, era, year, month, day, hour, minute, second, timeZoneName, formatMatcher] and [weekday, year, month, day, hour, minute, second, localeMatcher, hour12, hourCycle, timeZone, weekday, era, year, month, day, hour, minute, second, timeZoneName, formatMatcher] to have the same contents. FAILURE! intl402/DateTimeFormat/constructor-options-order.js Checks the order of getting options for the DateTimeFormat constructor. Error: Expected [localeMatcher, hour12, hourCycle, timeZone, weekday, era, year, month, day, hour, minute, second, timeZoneName, formatMatcher] and [weekday, year, month, day, hour, minute, second, localeMatcher, hour12, hourCycle, timeZone, weekday, era, year, month, day, hour, minute, second, timeZoneName, formatMatcher] to have the same contents. FAILURE! intl402/DateTimeFormat/constructor-options-order-fractionalSecondDigits.js Checks the order of getting options of 'fractionalSecondDigits' for the DateTimeFormat constructor. Error: Expected [localeMatcher, second, fractionalSecondDigits, timeZoneName, formatMatcher] and [second, fractionalSecondDigits, localeMatcher, second, fractionalSecondDigits, timeZoneName, formatMatcher] to have the same contents. FAILURE! intl402/DateTimeFormat/constructor-options-order-timedate-style.js Checks the order of getting options for the DateTimeFormat constructor. (Strict Mode) Error: Expected [localeMatcher, hour12, hourCycle, timeZone, weekday, era, year, month, day, hour, minute, second, timeZoneName, formatMatcher, dateStyle, timeStyle] and [weekday, year, month, day, hour, minute, second, dateStyle, timeStyle, localeMatcher, hour12, hourCycle, timeZone, weekday, era, year, month, day, hour, minute, second, timeZoneName, formatMatcher, dateStyle, timeStyle] to have the same contents. [00:04|: 342|+ 314|- 8|» 20] (70.00/s) FILES intl402/DateTimeFormat/constructor-options-order-dayPeriod.js intl402/DateTimeFormat/constructor-options-order-fractionalSecondDigits.js intl402/DateTimeFormat/constructor-options-order-timedate-style.js intl402/DateTimeFormat/constructor-options-order.js ```

anba · 2023-01-16T11:05:52Z

EDIT: But I'm also fine with not reordering, since it may not survive #747 anyway.

Yeah, let's do any additional reordering as part of #747. That way this PR is restricted to fix the TOCTU issue described in #237.

sffc · 2023-04-07T01:29:14Z

TG2 agreement: https://github.com/tc39/ecma402/blob/master/meetings/notes-2023-04-06.md#normative-read-date-time-options-only-once-when-creating-datetimeformat-objects-709

anba · 2023-06-29T05:22:48Z

Hi @ryzokuken, I don't see this issue being discussed at the May meeting. Do you plan to present it at the July meeting?

Thanks,
André

ryzokuken · 2023-06-29T11:13:29Z

Hi @anba! I indeed missed the fact that there's consensus here. I'll present it in the July meeting.

ryzokuken · 2023-07-21T19:40:35Z

This got approval at TG1, merging after merge conflict is fixed.

FrankYFTang · 2023-07-21T21:27:27Z

The following tests in test262 need to be updated to match this PR

'intl402/DateTimeFormat/constructor-options-order-dayPeriod': [FAIL],
'intl402/DateTimeFormat/constructor-options-order': [FAIL],
'intl402/DateTimeFormat/constructor-options-order-fractionalSecondDigits': [FAIL],
'intl402/DateTimeFormat/constructor-options-order-timedate-style': [FAIL],

ECMA402 PR709 remove unnecessary duplicated GetOption calls This PR sync the test to PR709 PR709 recach TC39 consensus in July 2023 meeting. tc39/ecma402#709

FrankYFTang · 2023-07-21T22:42:04Z

Fix tests in tc39/test262#3879

anba · 2023-07-22T06:26:40Z

The following tests in test262 need to be updated to match this PR

The test updates are in tc39/test262#3768.

FrankYFTang · 2023-07-24T21:19:44Z

please rebase, resolve conflict and merge. Thanks

justingrant · 2023-07-27T17:10:45Z

@ryzokuken - why is there a merge commit in this PR?

ryzokuken · 2023-07-27T17:11:41Z

@justingrant because I tried giving the GitHub interface a chance :/ Will rebase manually next time.

ljharb · 2023-07-27T17:26:44Z

@ryzokuken for future reference, there's an arrow next to "update branch" that lets you select "rebase branch", and it should work fine from the UI.

ryzokuken · 2023-07-27T17:31:36Z

After a lot of confusion, I realized that part of my troubles fixing up the merge issues was that the commits were already in the main branch? 02bd03a. Closing the PR, phew.

…ateTimeFormat objects" Update tests for PR tc39/ecma402#709.

tc39/ecma402#709 Change DateTimeFormat logic to read option only once. Track which fields is not undefined from the GetOptions. Change test/intl to reflect the change and remove dup test. PR709 reached TC39 consensus in July 2023 meeting. Bug: v8:13908 Change-Id: I5a5429f6375a01b04e77093d32acb76ae1e40d11 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4706795 Commit-Queue: Frank Tang <ftang@chromium.org> Reviewed-by: Shu-yu Guo <syg@chromium.org> Cr-Commit-Position: refs/heads/main@{#89241}

…DateTimeFormat objects. r=dminor Implements the changes from <tc39/ecma402#709>. The `intl_DateTimeFormat` self-hosted function is replaced with `intl_CreateDateTimeFormat`, which implements the new `CreateDateTimeFormat` abstract operation from the spec. `intl_CreateDateTimeFormat` calls into the existing `InitializeDateTimeFormat` function to actually initialise the newly created date-time formatter. Differential Revision: https://phabricator.services.mozilla.com/D189539

…DateTimeFormat objects. r=dminor Implements the changes from <tc39/ecma402#709>. The `intl_DateTimeFormat` self-hosted function is replaced with `intl_CreateDateTimeFormat`, which implements the new `CreateDateTimeFormat` abstract operation from the spec. `intl_CreateDateTimeFormat` calls into the existing `InitializeDateTimeFormat` function to actually initialise the newly created date-time formatter. Differential Revision: https://phabricator.services.mozilla.com/D189539 UltraBlame original commit: c1042546a44b0a4c0bcd37a6f3a6e6c6070a4cff

romulocintra requested review from ryzokuken, gibson042 and sffc September 23, 2022 09:38

sffc requested review from FrankYFTang and removed request for sffc September 23, 2022 22:11

gibson042 approved these changes Sep 24, 2022

View reviewed changes

spec/datetimeformat.html Outdated Show resolved Hide resolved

spec/datetimeformat.html Show resolved Hide resolved

gibson042 changed the title ~~Read date-time options only once when creating DateTimeFormat objects~~ Normative: Read date-time options only once when creating DateTimeFormat objects Sep 24, 2022

ryzokuken approved these changes Sep 27, 2022

View reviewed changes

gibson042 reviewed Oct 6, 2022

View reviewed changes

spec/datetimeformat.html Outdated Show resolved Hide resolved

ryzokuken added the normative label Nov 3, 2022

anba mentioned this pull request Jan 11, 2023

Should Intl APIs match Temporal interpretation of calendar and timeZone options? #741

Open

anba added a commit to anba/test262 that referenced this pull request Jan 12, 2023

Update tests for PR "Read date-time options only once when creating D…

e5c02da

…ateTimeFormat objects" Update tests for PR tc39/ecma402#709.

anba mentioned this pull request Jan 12, 2023

Update tests for PR "Read date-time options only once when creating DateTimeFormat objects" tc39/test262#3768

Merged

anba force-pushed the date-time-options-new branch from 742759f to 45c338b Compare January 12, 2023 13:45

justingrant reviewed Jan 12, 2023

View reviewed changes

spec/datetimeformat.html Outdated Show resolved Hide resolved

justingrant mentioned this pull request Jan 12, 2023

Align Temporal changes to 402 change: InitializeDateTimeFormat=>CreateDateTimeFormat tc39/proposal-temporal#2473

Closed

gibson042 added a commit to gibson042/engine262 that referenced this pull request Jan 14, 2023

Add a host-reliant Intl.DateTimeFormat

0e457ca

Ref tc39/ecma402#709 (comment)

gibson042 mentioned this pull request Jan 14, 2023

Can the order in which properties are read be more comprehensible? #747

Open

ryzokuken added the has consensus Has consensus from TC39-TG2 label Jun 29, 2023

FrankYFTang mentioned this pull request Jul 21, 2023

Remove unnecessary dup GetOption in DateTimeFormat to sync ECMA402PR709 tc39/test262#3879

Closed

Merge branch 'master' into date-time-options-new

ba2ee8e

ryzokuken closed this Jul 27, 2023

Ms2ger pushed a commit to anba/test262 that referenced this pull request Jul 27, 2023

Update tests for PR "Read date-time options only once when creating D…

a0b06b0

…ateTimeFormat objects" Update tests for PR tc39/ecma402#709.

Ms2ger pushed a commit to tc39/test262 that referenced this pull request Jul 27, 2023

Update tests for PR "Read date-time options only once when creating D…

21c3097

…ateTimeFormat objects" Update tests for PR tc39/ecma402#709.

anba mentioned this pull request Aug 4, 2023

ToDateTimeOptions should avoid making an observable internal-use object #706

Closed

anba mentioned this pull request Aug 14, 2023

Intl.DateTimeFormat calls getters on options objects twice #237

Closed

ptomato mentioned this pull request Jan 12, 2024

Update upstream specs tc39/proposal-temporal#2749

Merged

anba deleted the date-time-options-new branch June 20, 2024 16:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normative: Read date-time options only once when creating DateTimeFormat objects #709

Normative: Read date-time options only once when creating DateTimeFormat objects #709

anba commented Sep 20, 2022

sffc commented Oct 6, 2022

ryzokuken commented Nov 28, 2022

Ms2ger commented Jan 11, 2023

anba commented Jan 12, 2023

justingrant commented Jan 12, 2023

gibson042 commented Jan 14, 2023 •

edited

Loading

anba commented Jan 16, 2023

sffc commented Apr 7, 2023

anba commented Jun 29, 2023

ryzokuken commented Jun 29, 2023

ryzokuken commented Jul 21, 2023 •

edited

Loading

FrankYFTang commented Jul 21, 2023

FrankYFTang commented Jul 21, 2023

anba commented Jul 22, 2023

FrankYFTang commented Jul 24, 2023

justingrant commented Jul 27, 2023

ryzokuken commented Jul 27, 2023

ljharb commented Jul 27, 2023

ryzokuken commented Jul 27, 2023

Normative: Read date-time options only once when creating DateTimeFormat objects #709

Normative: Read date-time options only once when creating DateTimeFormat objects #709

Conversation

anba commented Sep 20, 2022

sffc commented Oct 6, 2022

ryzokuken commented Nov 28, 2022

Ms2ger commented Jan 11, 2023

anba commented Jan 12, 2023

justingrant commented Jan 12, 2023

gibson042 commented Jan 14, 2023 • edited Loading

anba commented Jan 16, 2023

sffc commented Apr 7, 2023

anba commented Jun 29, 2023

ryzokuken commented Jun 29, 2023

ryzokuken commented Jul 21, 2023 • edited Loading

FrankYFTang commented Jul 21, 2023

FrankYFTang commented Jul 21, 2023

anba commented Jul 22, 2023

FrankYFTang commented Jul 24, 2023

justingrant commented Jul 27, 2023

ryzokuken commented Jul 27, 2023

ljharb commented Jul 27, 2023

ryzokuken commented Jul 27, 2023

gibson042 commented Jan 14, 2023 •

edited

Loading

ryzokuken commented Jul 21, 2023 •

edited

Loading