Skip to content
This repository was archived by the owner on Apr 14, 2021. It is now read-only.

Don't override .toLocaleString functions if they're already wired up to native Intl #58

Closed
andyearnshaw opened this issue Feb 3, 2014 · 4 comments

Comments

@andyearnshaw
Copy link
Owner

Currently, we avoid overriding Intl by defining as IntlPolyfill. However, Number.prototype.toLocaleString, Date.prototype.toLocaleString and friends are still defined by our library and take the place of the native implementations.

We should check that these functions are not already wired up to a native implementation of Intl before defining them ourselves.

@caridy
Copy link
Collaborator

caridy commented Feb 6, 2014

About this particular issue, I was talking to @ericf while integrating the polyfills into Flickr last week, and it seems that we made a bad decision on IntlPolyfill. It seems that if we are changing Number and Date prototypes when posible, we should be also defining Intl when posible. It facilitates greatly the adoption, and if there is a way were Intl is in place, we could always add the IntlPolyfill for users who want to bypass the native implementation for whatever reason (normally bugs in native, which are very uncommon for ever-green browsers). What do you guys think?

@andyearnshaw
Copy link
Owner Author

So, you're saying define Intl in environments that don't have it native, but IntlPolyfill otherwise? That sounds like a decent enough proposal. As I've said before, I would like to make using Intl.js as transparent as possible, so that coders don't need to adjust their code to allow for it.

Generally, people should be advised to include it only if they need it. I'd even be fine with overriding native Intl if Intl.js is included, but for some reason the v8 team decided to make Intl non-configurable, which is one of the reasons for us going with IntlPolyfill.

@caridy
Copy link
Collaborator

caridy commented Feb 6, 2014

Yeah, my concrete proposal is:

  1. always set global.IntlPolyfill = factory();
  2. conditionally set global.Intl = global.IntlPolyfill;

that will cover the case of transparent inserting the polyfill in the page (conditionally or not), but also the case where you want to use the polyfill over the native implementation for a particular runtime.

@ant7
Copy link

ant7 commented Mar 13, 2014

It seems that the toLocaleString method is only overriden if the browser already has a native Intl implementation. I guess it should be the other way around.
Thank you

andyearnshaw added a commit that referenced this issue Mar 25, 2014
caridy added a commit that referenced this issue Dec 7, 2015
- Include short, long and full templates from CLDR as part of the extraction task to combine time and date formats correctly
  based on http://unicode.org/reports/tr35/tr35-dates.html#availableFormats_appendItems

- Diverge from spec becuase of bug #58 in ECMA 402 to requested options can overrule best match format as described here:
  tc39/ecma402#58

- Refactor the whole CLDR script to complain with the algo described here:
  http://unicode.org/reports/tr35/tr35-dates.html#availableFormats_appendItems
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants