Intent to Implement and Ship: rename Intl.DateTimeFormat.prototype.formatToParts type "dayperiod" to "dayPeriod"

89 views
Skip to first unread message

PhistucK

unread,
Jul 20, 2018, 11:56:46 AM7/20/18
to v8-users, blink-dev

(Probably an overkill, but here it goes)


Contact emails

phis...@gmail.com


Explainer

No explainer, a specification exists already.


Spec


Summary

This change corrects a non-compliant type value in the formatToParts implementation.


> new Intl.DateTimeFormat("en-us", {hour12: true, hour: "numeric"}).formatToParts()

[{"type": "hour", "value": "6"}, {"type": "literal", "value": " "}, {"type": "dayperiod", "value": "PM"}]


Will change to -

[{"type": "hour", "value": "6"}, {"type": "literal", "value": " "}, {"type": "dayPeriod", "value": "PM"}]


Motivation

Compliance with the standards and other browsers and likely most of the code that is already out there.


Risks

Interoperability and Compatibility

Compatibility risk - small to medium at worst.


Searched GitHub (not exhaustive, but some indication) for dayperiod instances -

The vast majority are polyfills that use dayPeriod already, or code that uses type.toLowerCase() to bridge over the differences.


Sent pull requests to the few cases that were plain wrong -


Interoperability risk - none.


Edge: No signals

Firefox: Shipped

Safari: Shipped

Web developers: No signals.


Alternatives for web developers

Either check for type === "dayPeriod" || type === "dayperiod", or type.toLowerCase() === "dayperiod".


Ergonomics

Irrelevant.


Activation

Irrelevant.


Debuggability

Already debuggable.


Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes.


Is this feature fully tested by web-platform-tests?

Nope, but it is tested by test262, though not this case (which is apparently why the interoperability issue exists).

I submitted a test262 pull request to maintain interoperability -

Requesting approval to ship?

Yes. I think so. Do you think a deprecation period is warranted? There is no (public?) use counter for formatToParts.



PhistucK

Philip Jägenstedt

unread,
Jul 20, 2018, 12:55:18 PM7/20/18
to PhistucK, v8-u...@googlegroups.com, blink-dev
This is an FYI for blink-dev, right? Should I remove it from https://bit.ly/blinkintents?

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABc02_%2B1xEoNvCtuc4ocTw%3DtLmfHmT-z-cFTuiE6YS9Q_MdPqg%40mail.gmail.com.

PhistucK

unread,
Jul 20, 2018, 1:00:30 PM7/20/18
to Philip Jägenstedt, v8-users, blink-dev
Guess so, though I am personally not going through with the change if there are any blink-dev reservations. ;)

PhistucK

Chris Harrelson

unread,
Jul 20, 2018, 2:24:48 PM7/20/18
to PhistucK, Philip Jägenstedt, v8-users, blink-dev
On Fri, Jul 20, 2018 at 10:00 AM PhistucK <phis...@gmail.com> wrote:
Guess so, though I am personally not going through with the change if there are any blink-dev reservations. ;)

Ok. You will need an LGTM from a v8 representative though. They will respond soon.
 
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABc02_KEi9ifQ6ExJwzFzYpj29gVvcd90CRX9RyDQrt2X%2BVbEQ%40mail.gmail.com.

PhistucK

unread,
Jul 20, 2018, 6:28:24 PM7/20/18
to Chris Harrelson, Philip Jägenstedt, v8-users, blink-dev
Yep, I know. No rush.

PhistucK

Adam Klein

unread,
Jul 26, 2018, 12:59:17 PM7/26/18
to PhistucK, v8-users, blink-dev, js...@chromium.org, Dan Ehrenberg
+Jungshik and Dan, who I believe worked on this feature in V8 originally. I'm curious if they know how it happened that this ended up with the wrong capitalization.

I appreciate the outreach you've done to fix uses in the wild, but it still scares me a little bit to make such a hard-breaking change, especially for V8-only environments like Node. So I'd also like to get some of your (or Jungshik or Dan's) intuition about how often this particular field is accessed.

--

PhistucK

unread,
Jul 26, 2018, 5:14:28 PM7/26/18
to Adam Klein, v8-users, blink-dev, Jungshik Shin, Daniel Ehrenberg
I misremembered formatToParts as a relatively recent feature, but now I see that the intent I remembered was actually discussing NumberFormat. DateTimeFormat did not seem to have an intent on blink-dev (but I see that it did on v8-users).
https://www.chromestatus.com/features/6319456309477376 says it is still behind a flag... Is the MDN article correct in stating that it was supported in Chrome 57 (and confusingly, also behind the flag until Chrome 60)?

Shameless plug - probably a good opportunity to add it to my filtered feed scraper and to my RSS reader -

Nevertheless, my intuition (more like a hunch, though) is that this specific field is relatively little used, but I may be wrong (the fact that my locale does not use it probably makes me biased).
From seeing all of the polyfills (which already use the standard casing) on GitHub (the search yielded a lot of those), I presumed they are also used by projects, which might mean those projects probably tested their polyfilled implementation as well on Internet Explorer 11 or Safari pre-11, so they would have probably seen the casing issue if they did something with that particular field (Salesforce worked around it, for example).
However, there is probably a lot of code that is Chrome only (:(), so...


Is there an option to add a use counter to V8?
Is there an existing use counter for formatToParts altogether that I may have missed (or maybe it is internal)?


Also, Node does not have to use V8 anymore, just saying. ;)

PhistucK

PhistucK

unread,
Jul 31, 2018, 4:40:44 AM7/31/18
to Adam Klein, v8-users, blink-dev, Jungshik Shin, Daniel Ehrenberg
Thoughts, anyone?

PhistucK

Adam Klein

unread,
Aug 7, 2018, 8:29:44 PM8/7/18
to PhistucK, v8-users, blink-dev, js...@chromium.org, Dan Ehrenberg
Apologies for the delay, this got hidden from me due to some gmail filtering issues...comments inline.

On Thu, Jul 26, 2018 at 2:14 PM PhistucK <phis...@gmail.com> wrote:
I misremembered formatToParts as a relatively recent feature, but now I see that the intent I remembered was actually discussing NumberFormat. DateTimeFormat did not seem to have an intent on blink-dev (but I see that it did on v8-users).
https://www.chromestatus.com/features/6319456309477376 says it is still behind a flag... Is the MDN article correct in stating that it was supported in Chrome 57 (and confusingly, also behind the flag until Chrome 60)?

Indeed, Chrome 57 looks like the right release (from looking through v8 commits). I've updated that on chromestatus. That is indeed awhile ago.
 
Shameless plug - probably a good opportunity to add it to my filtered feed scraper and to my RSS reader -

Nevertheless, my intuition (more like a hunch, though) is that this specific field is relatively little used, but I may be wrong (the fact that my locale does not use it probably makes me biased).
From seeing all of the polyfills (which already use the standard casing) on GitHub (the search yielded a lot of those), I presumed they are also used by projects, which might mean those projects probably tested their polyfilled implementation as well on Internet Explorer 11 or Safari pre-11, so they would have probably seen the casing issue if they did something with that particular field (Salesforce worked around it, for example).
However, there is probably a lot of code that is Chrome only (:(), so...

Again, it'd be great to get Jungshik's input on this, since he was the one who implemented it.
 
Is there an option to add a use counter to V8?
Is there an existing use counter for formatToParts altogether that I may have missed (or maybe it is internal)?

It is possible to add use counters to V8. They need to be plumbed through the API, and then manually updated from V8, so it's more work to add than it is in Blink, but it's doable. Would you be interested in adding such a counter?

PhistucK

unread,
Aug 8, 2018, 12:36:37 AM8/8/18
to Adam Klein, Jungshik Shin, Daniel Ehrenberg, v8-users, blink-dev
Comments inline.

PhistucK


On Wed, Aug 8, 2018 at 3:29 AM Adam Klein <ad...@chromium.org> wrote:
Apologies for the delay, this got hidden from me due to some gmail filtering issues...comments inline.

On Thu, Jul 26, 2018 at 2:14 PM PhistucK <phis...@gmail.com> wrote:
I misremembered formatToParts as a relatively recent feature, but now I see that the intent I remembered was actually discussing NumberFormat. DateTimeFormat did not seem to have an intent on blink-dev (but I see that it did on v8-users).
https://www.chromestatus.com/features/6319456309477376 says it is still behind a flag... Is the MDN article correct in stating that it was supported in Chrome 57 (and confusingly, also behind the flag until Chrome 60)?

Indeed, Chrome 57 looks like the right release (from looking through v8 commits). I've updated that on chromestatus. That is indeed awhile ago.

Thank you.

 
 
Shameless plug - probably a good opportunity to add it to my filtered feed scraper and to my RSS reader -

Nevertheless, my intuition (more like a hunch, though) is that this specific field is relatively little used, but I may be wrong (the fact that my locale does not use it probably makes me biased).
From seeing all of the polyfills (which already use the standard casing) on GitHub (the search yielded a lot of those), I presumed they are also used by projects, which might mean those projects probably tested their polyfilled implementation as well on Internet Explorer 11 or Safari pre-11, so they would have probably seen the casing issue if they did something with that particular field (Salesforce worked around it, for example).
However, there is probably a lot of code that is Chrome only (:(), so...

Again, it'd be great to get Jungshik's input on this, since he was the one who implemented it.

I agree, it would be great if you pinged Jungshik on Hangouts or something and ask Jungshik to follow this thread...

 
 
Is there an option to add a use counter to V8?
Is there an existing use counter for formatToParts altogether that I may have missed (or maybe it is internal)?

It is possible to add use counters to V8. They need to be plumbed through the API, and then manually updated from V8, so it's more work to add than it is in Blink, but it's doable. Would you be interested in adding such a counter?

It is probably much more than I bargained for (especially the delay until we get results and usage can increase by the day). ;) But if you have a change list I can follow for guidance, I might just do that (barring a positive response from Jungshik).

js...@chromium.org

unread,
Aug 25, 2018, 2:23:06 AM8/25/18
to blink-dev, ad...@chromium.org, js...@chromium.org, litt...@chromium.org, v8-u...@googlegroups.com
Sorry for the delay as well as for the stupid typo I made in 'dayPeriod'. I've been ooo for a while and catching up with emails. 


On Tuesday, August 7, 2018 at 9:36:37 PM UTC-7, PhistucK wrote:
Comments inline.

PhistucK


On Wed, Aug 8, 2018 at 3:29 AM Adam Klein <ad...@chromium.org> wrote:
Apologies for the delay, this got hidden from me due to some gmail filtering issues...comments inline.

On Thu, Jul 26, 2018 at 2:14 PM PhistucK <phis...@gmail.com> wrote:
I misremembered formatToParts as a relatively recent feature, but now I see that the intent I remembered was actually discussing NumberFormat. DateTimeFormat did not seem to have an intent on blink-dev (but I see that it did on v8-users).
https://www.chromestatus.com/features/6319456309477376 says it is still behind a flag... Is the MDN article correct in stating that it was supported in Chrome 57 (and confusingly, also behind the flag until Chrome 60)?

Indeed, Chrome 57 looks like the right release (from looking through v8 commits). I've updated that on chromestatus. That is indeed awhile ago.

Thank you.

Yeah. Chrome 57 contained it behind a flag and Chrome 60 began to have it by default.  

 
 
Shameless plug - probably a good opportunity to add it to my filtered feed scraper and to my RSS reader -

Nevertheless, my intuition (more like a hunch, though) is that this specific field is relatively little used, but I may be wrong (the fact that my locale does not use it probably makes me biased).
From seeing all of the polyfills (which already use the standard casing) on GitHub (the search yielded a lot of those), I presumed they are also used by projects, which might mean those projects probably tested their polyfilled implementation as well on Internet Explorer 11 or Safari pre-11, so they would have probably seen the casing issue if they did something with that particular field (Salesforce worked around it, for example).
However, there is probably a lot of code that is Chrome only (:(), so...

Again, it'd be great to get Jungshik's input on this, since he was the one who implemented it.

I agree, it would be great if you pinged Jungshik on Hangouts or something and ask Jungshik to follow this thread...

There'd be no worry at all if there were NO Chrome-only-implementation.   My wild speculation (with no material basis to support)  is that those who write Chrome-only code is less likely to be aware of and to utilize EcmaScript Intl API. Alternatively, my hunch is that  those who use Intl API  (especially this relatively new API) are pretty likely to care about cross-browser compatibility and work around v8's typo (my typo) in this field. 

formatToParts API is mainly for controlling the style of individual parts separately. If the case of 'dayPeriod' is fixed in v8 and does not match Chrome-only-code's 'dayperiod' any more, locales using dayPeriod (AM/PM marker) would have a wrong style (font size, color, etc) in Chrome ToT, but the information will be still there. 

Given the above and the fact that a work-around (use case-insensitive match or check for both case-variants) is simple,  I'd say we go ahead with this change.  Adding a counter to measure the usage seems to be a bit too much. 

Jungshik  

Adam Klein

unread,
Aug 28, 2018, 1:25:26 PM8/28/18
to js...@chromium.org, blink-dev, Dan Ehrenberg, v8-users
Thanks, Jungshik. Based on this feedback I'm comfortable with making this change without adding a usecounter.

Daniel Ehrenberg

unread,
Sep 6, 2018, 1:00:06 PM9/6/18
to Adam Klein, js...@chromium.org, blink-dev, v8-users
I'm comfortable with this change as well. A relatively new API like this, which is implemented compliantly in another browser, seems likely to have relatively low compatibility risk. It will still be some time until these new Intl features gain uptake to replace JS libraries like Globalize that serve the same purpose. Great catch, PhistucK, and thanks for following up so thoroughly in this report.

Dan

PhistucK

unread,
Sep 6, 2018, 2:47:23 PM9/6/18
to Daniel Ehrenberg, Adam Klein, Jungshik Shin, blink-dev, v8-users
Great catch goes to han.guokai@ which filed the original issue. :)

By the way, all of the pull requests were merged, so mostly accidentally old code would be broken at this point (unless new cases were added since then :().

I will work on the change list again soon.

PhistucK


To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
Reply all
Reply to author
Forward
0 new messages