benchmark.js icon indicating copy to clipboard operation
benchmark.js copied to clipboard

feat: use Number.toLocaleString to support localization

Open pimlie opened this issue 6 years ago • 5 comments

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 :)

pimlie avatar Jan 02 '19 19:01 pimlie

Thank you @pimlie!

It looks like related unit tests are failing. Could you double check them please.

jdalton avatar Jan 02 '19 20:01 jdalton

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?

pimlie avatar Jan 02 '19 22:01 pimlie

Dropping phantom is fine 😄

jdalton avatar Jan 02 '19 23:01 jdalton

Great thanks, even better :+1:

Let me know if you have any other remarks!

pimlie avatar Jan 02 '19 23:01 pimlie

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...

prantlf avatar Jan 31 '20 11:01 prantlf