The formatRange function for Intl.DateTimeFormat is at Stage 3 in TC39; the proposal is on track to reach Stage 4 in 2020. V8 has had it ready for some time, and SpiderMonkey plans to add it in the first half of this year. As usage of formatRange in Intl.DateTimeFormat increases throughout the ecosystem, WebKit users will be left behind with legacy polyfills, leading to broken web sites and/or poorer performance relative to other browsers. At Google, we are currently weighing our options for calling these features of formatRange in Intl.DateTimeFormat in supported environments in order to give users better performance and smaller download sizes. ICU4C exposes C and C++ APIs that can be used to implement formatRange in Intl.DateTimeFormat. Implementing formatRange in Intl.DateTimeFormat is largely a matter of adding the glue between JavaScript and ICU4C, as WebKit already does for other Intl types. Proposal repo: https://github.com/fabalbon/proposal-intl-DateTimeFormat-formatRange
<rdar://problem/61079767>
Use UDateIntervalFormat to implement this. https://unicode-org.github.io/icu-docs/apidoc/released/icu4c/udateintervalformat_8h.html
formatRangeToParts requires ICU API (udtitvfmt_formatToResult ) which is added in 67 and it is draft state. Possibly we will skip it.
Spawned https://bugs.webkit.org/show_bug.cgi?id=213822
Created attachment 403255 [details] Patch WIP
Created attachment 403260 [details] Patch
Comment on attachment 403260 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403260&action=review > Source/JavaScriptCore/runtime/IntlDateTimeFormat.cpp:649 > + dataLogLnIf(IntlDateTimeFormatInternal::verbose, "locale:(", m_locale, "),dataLocale:(", dataLocaleWithExtensions, "),pattern:(", pattern, ")"); I feel kind of unclear about when these verbose logs should actually be landed, but I don't object if you think it'll continue to be useful. > Source/JavaScriptCore/runtime/IntlDateTimeFormat.cpp:1054 > + // http://tc39.es/proposal-intl-DateTimeFormat-formatRange/#sec-partitiondatetimerangepattern Should this be at the top of the function for consistency? > JSTests/ChangeLog:13 > + * stress/intl-datetimeformat-formatrange-relevant-extensions.js: Added. Seems a little arbitrary that this is split out into a separate file, but glad to see we've tested various options and extensions. I think we need to cover a non-English locale too though. > JSTests/test262/expectations.yaml:1612 > + default: 'Test262Error: no args Expected a RangeError but got a TypeError' Is this one really related to ICU version? We're throwing the wrong type of error for `new Intl.DateTimeFormat().formatRange()`.
Comment on attachment 403260 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403260&action=review >> Source/JavaScriptCore/runtime/IntlDateTimeFormat.cpp:649 >> + dataLogLnIf(IntlDateTimeFormatInternal::verbose, "locale:(", m_locale, "),dataLocale:(", dataLocaleWithExtensions, "),pattern:(", pattern, ")"); > > I feel kind of unclear about when these verbose logs should actually be landed, but I don't object if you think it'll continue to be useful. Yeah, this is very useful to check what locale is passed and what pattern is generated, since this is the input of ICU. When I see some problems, I check this first. If this is reasonable, then we need to look into ICU. If these inputs looks non-reasonable, then our frontend to ICU is not correct :) >> Source/JavaScriptCore/runtime/IntlDateTimeFormat.cpp:1054 >> + // http://tc39.es/proposal-intl-DateTimeFormat-formatRange/#sec-partitiondatetimerangepattern > > Should this be at the top of the function for consistency? I think here is better since this URL is not for formatRange, this is for PartitionDateTimeRangePattern abstract operation called by formatRange. >> JSTests/ChangeLog:13 >> + * stress/intl-datetimeformat-formatrange-relevant-extensions.js: Added. > > Seems a little arbitrary that this is split out into a separate file, but glad to see we've tested various options and extensions. > I think we need to cover a non-English locale too though. Right. I'll add some more. >> JSTests/test262/expectations.yaml:1612 >> + default: 'Test262Error: no args Expected a RangeError but got a TypeError' > > Is this one really related to ICU version? > We're throwing the wrong type of error for `new Intl.DateTimeFormat().formatRange()`. Oops, right!!! Fixed.
Comment on attachment 403260 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403260&action=review >>> JSTests/test262/expectations.yaml:1612 >>> + default: 'Test262Error: no args Expected a RangeError but got a TypeError' >> >> Is this one really related to ICU version? >> We're throwing the wrong type of error for `new Intl.DateTimeFormat().formatRange()`. > > Oops, right!!! Fixed. Oh, interesting. This is test262's bug. http://tc39.es/proposal-intl-DateTimeFormat-formatRange/#sec-intl.datetimeformat.prototype.formatRange "If startDate is undefined or endDate is undefined, throw a TypeError exception." 20 var dtf = new Intl.DateTimeFormat(); 21 22 assert.throws(RangeError, function() { 23 dtf.formatRange(); // Not possible to poison this one 24 }, "no args");
Created attachment 403266 [details] Patch WIP
Created attachment 403271 [details] Patch
Comment on attachment 403260 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403260&action=review >>>> JSTests/test262/expectations.yaml:1612 >>>> + default: 'Test262Error: no args Expected a RangeError but got a TypeError' >>> >>> Is this one really related to ICU version? >>> We're throwing the wrong type of error for `new Intl.DateTimeFormat().formatRange()`. >> >> Oops, right!!! Fixed. > > Oh, interesting. This is test262's bug. > http://tc39.es/proposal-intl-DateTimeFormat-formatRange/#sec-intl.datetimeformat.prototype.formatRange > "If startDate is undefined or endDate is undefined, throw a TypeError exception." > > 20 var dtf = new Intl.DateTimeFormat(); > 21 > 22 assert.throws(RangeError, function() { > 23 dtf.formatRange(); // Not possible to poison this one > 24 }, "no args"); Uploaded the PR https://github.com/tc39/test262/pull/2685
Created attachment 403272 [details] Patch
Comment on attachment 403272 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403272&action=review r=me. > JSTests/test262/expectations.yaml:1617 > +test/intl402/DateTimeFormat/prototype/formatRangeToParts/argument-date-string.js: Assuming we're not passing any tests in this directory, we should just add a path skip to config.yaml (with a comment linking to bug 213822).
Comment on attachment 403272 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403272&action=review >> JSTests/test262/expectations.yaml:1617 >> +test/intl402/DateTimeFormat/prototype/formatRangeToParts/argument-date-string.js: > > Assuming we're not passing any tests in this directory, we should just add a path skip to config.yaml (with a comment linking to bug 213822). Thats nice!
Created attachment 403275 [details] Patch for landing
Created attachment 403300 [details] Patch for landing
Created attachment 403304 [details] Patch for landing
Created attachment 403317 [details] Patch for landing
Created attachment 403319 [details] Patch for landing
Created attachment 403324 [details] Patch for landing
Committed r266033: <https://trac.webkit.org/changeset/266033>