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* On Thu, Sep 6, 2018 at 8:00 PM Daniel Ehrenberg <little...@chromium.org> wrote: > 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 > > On Tue, Aug 28, 2018 at 7:25 PM Adam Klein <ad...@chromium.org> wrote: > >> Thanks, Jungshik. Based on this feedback I'm comfortable with making this >> change without adding a usecounter. >> >> On Fri, Aug 24, 2018 at 11:23 PM <js...@chromium.org> wrote: >> >>> 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 <phist...@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 >>>>>> <https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DateTimeFormat/formatToParts> >>>>>> 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 - >>>>>> >>>>>> https://frequentfeedscraper.appspot.com/read?feed=v8users&title_filter=exact:intent >>>>>> >>>>>> 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 >>> >>> >>>> >>>> >>>>> >>>>> >>>>>> 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). >>>> >>>> >>>> >>>>> >>>>> >>>>>> Also, Node does not have to use V8 anymore, just saying. ;) >>>>>> >>>>>> ☆*PhistucK* >>>>>> >>>>>> >>>>>> On Thu, Jul 26, 2018 at 7:59 PM Adam Klein <ad...@chromium.org> >>>>>> wrote: >>>>>> >>>>>>> +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. >>>>>>> >>>>>>> On Fri, Jul 20, 2018 at 8:56 AM PhistucK <phist...@gmail.com> wrote: >>>>>>> >>>>>>>> (Probably an overkill, but here it goes) >>>>>>>> >>>>>>>> >>>>>>>> Contact emails >>>>>>>> >>>>>>>> phist...@gmail.com >>>>>>>> >>>>>>>> Explainer >>>>>>>> >>>>>>>> No explainer, a specification exists already. >>>>>>>> >>>>>>>> Spec >>>>>>>> https://tc39.github.io/ecma402/#sec-partitiondatetimepattern >>>>>>>> >>>>>>>> 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": "day*p*eriod", "value": "PM"}] >>>>>>>> >>>>>>>> >>>>>>>> Will change to - >>>>>>>> >>>>>>>> [{"type": "hour", "value": "6"}, {"type": "literal", "value": " "}, >>>>>>>> {"type": "day*P*eriod", "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 - >>>>>>>> >>>>>>>> https://github.com/search?l=&p=1&q=formatToParts+dayperiod+language%3AJavaScript&type=Code >>>>>>>> >>>>>>>> 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 - >>>>>>>> https://github.com/sensu/sensu-go/pull/1852 >>>>>>>> https://github.com/brave/browser-laptop/pull/14790 >>>>>>>> https://github.com/ray007/js-misc/pull/1 >>>>>>>> https://github.com/OriginalNexus/venture/pull/1 >>>>>>>> https://github.com/ua9msn/datetime/pull/9 >>>>>>>> >>>>>>>> >>>>>>>> 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 >>>>>>>> <https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_platform_tests.md> >>>>>>>> ? >>>>>>>> >>>>>>>> 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 -* >>>>>>>> *https://github.com/tc39/test262/pull/1645 >>>>>>>> <https://github.com/tc39/test262/pull/1645>* >>>>>>>> >>>>>>>> >>>>>>>> Bug and proposed change list - >>>>>>>> >>>>>>>> https://crbug.com/865351 >>>>>>>> >>>>>>>> https://chromium-review.googlesource.com/c/v8/v8/+/1145304 >>>>>>>> >>>>>>>> >>>>>>>> Link to entry on the feature dashboard >>>>>>>> <https://www.chromestatus.com/> >>>>>>>> https://www.chromestatus.com/features/5252265900244992 >>>>>>>> >>>>>>>> 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* >>>>>>>> >>>>>>>> -- >>>>>>>> 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 >>>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABc02_%2B1xEoNvCtuc4ocTw%3DtLmfHmT-z-cFTuiE6YS9Q_MdPqg%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>>>>> . >>>>>>>> >>>>>>> -- > You received this message because you are subscribed to the Google Groups > "blink-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to blink-dev+unsubscr...@chromium.org. > To view this discussion on the web visit > https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAKtSNYN28nuD1N1VBCjhae3Dvd9mB%3D--ah9%3DnESzg%3Dju6J%3DfEA%40mail.gmail.com > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAKtSNYN28nuD1N1VBCjhae3Dvd9mB%3D--ah9%3DnESzg%3Dju6J%3DfEA%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > -- -- v8-users mailing list v8-users@googlegroups.com http://groups.google.com/group/v8-users --- You received this message because you are subscribed to the Google Groups "v8-users" group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-users+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
