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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> wrote: >>>>> >>>>>> (Probably an overkill, but here it goes) >>>>>> >>>>>> >>>>>> Contact emails >>>>>> >>>>>> [email protected] >>>>>> >>>>>> 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> >>>>>> . >>>>>> >>>>> -- -- v8-users mailing list [email protected] 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 [email protected]. For more options, visit https://groups.google.com/d/optout.
