feat: use Number.toLocaleString to support localization
Resolves: #210
This PR add locale support by using Number.toLocaleString for number formatting.
Btw, as we discussed in the corresponding issue I did some work on adapting a polyfill for Number.toLocaleString but unfortunately I couldnt get it to fully work correctly in the time I had set for this (the polyfill works actually but when used as a dependency it doesnt download the correct cldr-data due to an issue in the cldr-data-npm package). As most (current?) browsers fully support number localization already I think the full-icu solution will work just fine :)
Thank you @pimlie!
It looks like related unit tests are failing. Could you double check them please.
Hi @jdalton,
PhantomJS doesnt seem to support toLocaleString (see their issue 12581, it just uses toString). Also the project is currently archived, so what solution do you prefer? Skip the numberFormat tests? Do a manual comparison on user-agent and use the old formatNumber code for phantomjs (and skip the numberFormat tests with a locale)?
The polyfill I adapted doesnt seem to work either when I hack it, it uses libraries which are not phantomjs compatible it seems.
Also I simplified the travis config a bit (although opinions may vary), it work nicely (except for the phantomjs issues) but before I 'finalize' that config maybe you are opposed against using npm scripts like this and prefer the old config?
Dropping phantom is fine 😄
Great thanks, even better :+1:
Let me know if you have any other remarks!
PhantomJS doesnt seem to support
toLocaleString(see their issue 12581, it just uses toString).
You could switch to Puppeteer. With some additional test-refactoring effort, of course...