search

rxaviers / cldrjs

Simple CLDR traverser
MIT License
155 stars 30 forks source link

IE8 Support #44

Closed ctolkien closed 7 years ago

ctolkien commented 8 years ago

Yes I know.. I know.

It is listed as one of the supported browsers however. Currently we're hitting errors as it does not support array.indexOf(). This was apparently added in IE9. This is being hit here:

https://github.com/rxaviers/cldrjs/blob/d862808e10477092fa4b41176471b957cd6224d5/src/core/set_available_bundles.js#L22

rxaviers commented 8 years ago

Thanks for filing this issue.

We could polyfill this method in a similar fashion to https://github.com/rxaviers/cldrjs/blob/master/src/util/array/for_each.js perhaps by using https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Array/indexOf#Polyfill as reference, but reducing it to our specific use case (e.g., input is always a non-empty String).

Would you like to work on that?

Having said that, it's time to consider dropping IE8 support (and drop several of those helper functions).

ctolkien commented 8 years ago

We've polyfilled it in for our own case. I'm in two minds on this one, on the one hand, it's entirely fair to just drop IE8 support. On the other hand, there's only this single method call preventing support. So I'm leaning towards the latter.

I'd be weary of polyfilling primitives in a library, so I think revising implementation is probably best.

Am off with a new bub at the moment, will keep this on the radar.

rxaviers commented 8 years ago

Just to be clear, this libraries doesn't polyfill any primitives, sorry for the confusion. We have helper functions like https://github.com/rxaviers/cldrjs/blob/master/src/util/array/for_each.js.

rxaviers commented 7 years ago

I believe we can safely ignore this support.