Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

About interop with dateStyle/timeStyle #19

Closed
sffc opened this issue Nov 27, 2019 · 7 comments
Closed

About interop with dateStyle/timeStyle #19

sffc opened this issue Nov 27, 2019 · 7 comments

Comments

@sffc
Copy link

sffc commented Nov 27, 2019

I wanted to suggest one possible resolution to ICU-20710. This is the feature request regarding ICU not supporting dateStyle/timeStyle in DateIntervalFormat.

I had suggested in CLDR-13425 that one resolution could be to map dateStyle/timeStyle to skeletons that could then be used to feed the DateIntervalFormat. However, I was reminded by @mihnita that dateStyle/timeStyle exists in large part because these are settings that come from the user's OS preferences, which are patterns, not skeletons.

This raises a question: should we just think about dateStyle/timeStyle as being completely orthogonal to all other Intl.DateTimeFormat features?

A semantic that would solve the outstanding issue would be simple: throw a TypeError in formatRange when dateStyle/timeStyle was used in the constructor.

Thoughts?
@zbraniecki
Copy link
Member

I'm in favor of CLDR-13425. I believe that the OSes mapping styles to patterns is possible a mistake.

@sffc
Copy link
Author

sffc commented Nov 27, 2019

I'm in favor of CLDR-13425. I believe that the OSes mapping styles to patterns is possible a mistake.

In a CLDR-13425 world, what would implementations do when the dateStyle/timeStyle patterns come from the OS instead of from CLDR?

I'm not completely familiar with the mechanics of OS-based dateStyle/timeStyle, but it seems that if this is long-standing precedent that we want to tap into, we're a bit compelled to go with the suboptimal pattern-based solution, and if we throw exceptions, we can always change that behavior if OSes migrate to skeletons for user preferences in the future.

@sffc
Copy link
Author

sffc commented Nov 27, 2019

Related: tc39/proposal-intl-datetime-style#2 (comment)

@sffc
Copy link
Author

sffc commented Dec 12, 2019

Chrome 79 behavior:

new Intl.DateTimeFormat("en", { dateStyle: "long", timeStyle: "short" }).format(new Date())
// "December 12, 2019 at 11:48 AM"
new Intl.DateTimeFormat("en", { dateStyle: "long", timeStyle: "short" }).formatRange(new Date(), new Date())
// "December 12, 2019, 11:48 AM"

@sffc
Copy link
Author

sffc commented Dec 12, 2019

Conclusion from ECMA-402:

V8's workaround behavior is OK. Consider either updating the spec to allow it, or leave it as a known issue for compliance to be resolved when ICU/CLDR eventually add this feature.

@sffc
Copy link
Author

sffc commented Oct 8, 2020

Conclusion 2020-10-08: keep the spec the same and test it in Test262.

@sffc
Copy link
Author

sffc commented Oct 8, 2020

2020-10-08 discussion: https://github.com/tc39/ecma402/blob/master/meetings/notes-2020-10-08.md#about-interop-with-datestyletimestyle-19

fabalbon added a commit to fabalbon/test262 that referenced this issue Dec 28, 2020
@fabalbon
Add tests for Intl.DateTimeFormat formatRange(ToParts) to check the b…
122916e 
…ehavior when startDate is the same as endDate (tc39/proposal-intl-DateTimeFormat-formatRange#19).
@fabalbon fabalbon mentioned this issue Dec 28, 2020
Merged
rwaldron pushed a commit to tc39/test262 that referenced this issue Jan 4, 2021
@fabalbon @rwaldron
Add tests for Intl.DateTimeFormat formatRange(ToParts) to check the b…
22cdb74 
…ehavior when startDate is the same as endDate (tc39/proposal-intl-DateTimeFormat-formatRange#19).
@fabalbon fabalbon closed this as completed Jan 5, 2021
webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this issue Jan 7, 2021
@Constellation
[JSC] DateTimeFormat#formatRange should generate the same output to D…
0abeff7 
…ateTimeFormat#format if startDate and endDate are "practically-equal"

https://bugs.webkit.org/show_bug.cgi?id=220395

Reviewed by Ross Kirsling.

JSTests:

Imported some test262 tests. Updated, fixed some issues (`formatToParts` test was using `format`, we should upstream it to test262), and added more tests.

* stress/intl-datetimeformat-format-range-should-check-practically-equal.js: Added.
(shouldBe):
(vm.icuVersion):
* stress/intl-datetimeformat-format-range-to-parts-should-check-practically-equal.js: Added.
(shouldBe):
(zip):
(compare):
(vm.icuVersion):

Source/JavaScriptCore:

Intl.DateTimeFormat.formatRange(startDate, endDate) also needs to generate the same formatted string to the Intl.DateTimeFormat.format
if startDate and endDate are *practically-equal* (spec term). However, due to CLDR, just using udtitvfmt_format generates different
formatted string to udat_format's result even though startDate and endDate are the same.

    new Intl.DateTimeFormat("en", { dateStyle: "long", timeStyle: "short" }).format(new Date())
    // "December 12, 2019 at 11:48 AM"
    new Intl.DateTimeFormat("en", { dateStyle: "long", timeStyle: "short" }).formatRange(new Date(), new Date())
    // "December 12, 2019, 11:48 AM"

In Intl.DateTimeFormat#formatRangeToParts, we deploys *practically-equal* checking to avoid this issue. The same thing should be done in
Intl.DateTimeFormat#formatRange too.

In this patch, we stop using udtitvfmt_format if ICU version is 64 or later to perform *practically-equal* checking.

[1]: tc39/proposal-intl-DateTimeFormat-formatRange#19

* runtime/IntlDateTimeFormat.cpp:
(JSC::formattedValueFromDateRange):
(JSC::dateFieldsPracticallyEqual):
(JSC::IntlDateTimeFormat::formatRange):
(JSC::IntlDateTimeFormat::formatRangeToParts):
(JSC::definitelyAfterGregorianCalendarChangeDate): Deleted.

Canonical link: https://commits.webkit.org/232820@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@271224 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima pushed a commit to caiolima/webkit that referenced this issue Jan 11, 2021
[JSC] DateTimeFormat#formatRange should generate the same output to D…
8a6e879 
…ateTimeFormat#format if startDate and endDate are "practically-equal"

https://bugs.webkit.org/show_bug.cgi?id=220395

Reviewed by Ross Kirsling.

JSTests:

Imported some test262 tests. Updated, fixed some issues (`formatToParts` test was using `format`, we should upstream it to test262), and added more tests.

* stress/intl-datetimeformat-format-range-should-check-practically-equal.js: Added.
(shouldBe):
(vm.icuVersion):
* stress/intl-datetimeformat-format-range-to-parts-should-check-practically-equal.js: Added.
(shouldBe):
(zip):
(compare):
(vm.icuVersion):

Source/JavaScriptCore:

Intl.DateTimeFormat.formatRange(startDate, endDate) also needs to generate the same formatted string to the Intl.DateTimeFormat.format
if startDate and endDate are *practically-equal* (spec term). However, due to CLDR, just using udtitvfmt_format generates different
formatted string to udat_format's result even though startDate and endDate are the same.

    new Intl.DateTimeFormat("en", { dateStyle: "long", timeStyle: "short" }).format(new Date())
    // "December 12, 2019 at 11:48 AM"
    new Intl.DateTimeFormat("en", { dateStyle: "long", timeStyle: "short" }).formatRange(new Date(), new Date())
    // "December 12, 2019, 11:48 AM"

In Intl.DateTimeFormat#formatRangeToParts, we deploys *practically-equal* checking to avoid this issue. The same thing should be done in
Intl.DateTimeFormat#formatRange too.

In this patch, we stop using udtitvfmt_format if ICU version is 64 or later to perform *practically-equal* checking.

[1]: tc39/proposal-intl-DateTimeFormat-formatRange#19

* runtime/IntlDateTimeFormat.cpp:
(JSC::formattedValueFromDateRange):
(JSC::dateFieldsPracticallyEqual):
(JSC::IntlDateTimeFormat::formatRange):
(JSC::IntlDateTimeFormat::formatRangeToParts):
(JSC::definitelyAfterGregorianCalendarChangeDate): Deleted.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@271224 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@zbraniecki @sffc @fabalbon