Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

feat: use Number.toLocaleString to support localization #216

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
.coveralls.yml
.DS_Store
.nyc_output
*.log
coverage
node_modules
29 changes: 4 additions & 25 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,10 @@ node_js:
cache:
directories:
- ~/.npm
- travis_phantomjs

env:
global:
- BIN=node ISTANBUL=false
- secure: XOINlYZG3DYXq5agQXvkad2MfnOO/+z40fn37RKfNuxNI5veBK7tPRcCIQ998o+NyVTpyd3NZhqkowCxAL8bp4HJ81SesRKQSaXoSzgV7CnloxLxcduiiRJ6lnxFGgCbpqyLxZHWr0mQugcLhs5nhiZ5Dnw6dZxYX/oKKvOomZA=
matrix:
-
- ISTANBUL=true
- BIN=phantomjs

git:
depth: 10
Expand All @@ -28,26 +22,11 @@ branches:
- master

before_install:
# Upgrade PhantomJS.
- |
export PHANTOMJS_VERSION=2.1.1
export PATH=$PWD/travis_phantomjs/phantomjs-$PHANTOMJS_VERSION-linux-x86_64/bin:$PATH
if [ $(phantomjs --version) != $PHANTOMJS_VERSION ]; then
rm -rf $PWD/travis_phantomjs
mkdir -p $PWD/travis_phantomjs
wget https://github.com/Medium/phantomjs/releases/download/v$PHANTOMJS_VERSION/phantomjs-$PHANTOMJS_VERSION-linux-x86_64.tar.bz2
tar -xvf phantomjs-$PHANTOMJS_VERSION-linux-x86_64.tar.bz2 -C $PWD/travis_phantomjs
fi
phantomjs -v

- nvm use $TRAVIS_NODE_VERSION
- npm i -g npm

script:
- |
if [ $ISTANBUL = true ]; then
istanbul cover -x "**/vendor/**" --report lcovonly ./test/test.js && cat ./coverage/lcov.info | coveralls && rm -rf ./coverage
else
cd ./test
$BIN ./test.js
fi
- npm run test

after_success:
- npm run coverage
22 changes: 21 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ suite.add('RegExp#test', function() {
console.log('Fastest is ' + this.filter('fastest').map('name'));
})
// run async
.run({ 'async': true });
.run({ 'async': true, 'locale': 'en-US' });

// logs:
// => RegExp#test x 4,161,532 +-0.99% (59 cycles)
Expand All @@ -86,6 +86,26 @@ suite.add('RegExp#test', function() {

Tested in Chrome 54-55, Firefox 49-50, IE 11, Edge 14, Safari 9-10, Node.js 6-7, & PhantomJS 2.1.1.

## Localization

Results are formatted using [Number.prototype.toLocaleString](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toLocaleString)

### Node.js

Node.js does not support all number localizations by default. Install the optional [full-icu](https://www.npmjs.com/package/full-icu) package

```
npm i --save full-icu
```

and run your benchmark as follows:

```
NODE_ICU_DATA=./node_modules/full-icu node your_benchmark.js
```

Read the [Internationalization chapter](https://nodejs.org/dist/latest/docs/api/intl.html) from the Node.js documentation for more information.

## BestieJS

Benchmark.js is part of the BestieJS *“Best in Class”* module collection. This means we promote solid browser/environment support, ES5+ precedents, unit testing, & plenty of documentation.
26 changes: 20 additions & 6 deletions benchmark.js
Original file line number Diff line number Diff line change
Expand Up @@ -787,12 +787,17 @@
* @static
* @memberOf Benchmark
* @param {number} number The number to convert.
* @param {locale} string Identifying string of the locale to use.
* @returns {string} The more readable string representation.
*/
function formatNumber(number) {
number = String(number).split('.');
return number[0].replace(/(?=(?:\d{3})+$)(?!\b)/g, ',') +
(number[1] ? '.' + number[1] : '');
function formatNumber(number, locale) {
if (typeof Number.prototype.toLocaleString === 'function') {
return number.toLocaleString(locale);
} else {
number = String(number).split('.');
return number[0].replace(/(?=(?:\d{3})+$)(?!\b)/g, ',') +
(number[1] ? '.' + number[1] : '');
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a check during the creation of formatNumber that Number.prototype.toLocaleString is a function and if it's not then make formatNumber fallback to the legacy formatter?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is like this ok or did you mean something else with during the creation of formatNumber?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'll do!


/**
Expand Down Expand Up @@ -1527,6 +1532,7 @@
error = bench.error,
hz = bench.hz,
id = bench.id,
locale = bench.locale,
stats = bench.stats,
size = stats.sample.length,
pm = '\xb1',
Expand All @@ -1545,7 +1551,7 @@
result += ': ' + errorStr;
}
else {
result += ' x ' + formatNumber(hz.toFixed(hz < 100 ? 2 : 0)) + ' ops/sec ' + pm +
result += ' x ' + formatNumber(hz.toFixed(hz < 100 ? 2 : 0), locale) + ' ops/sec ' + pm +
stats.rme.toFixed(2) + '% (' + size + ' run' + (size == 1 ? '' : 's') + ' sampled)';
}
return result;
Expand Down Expand Up @@ -2262,7 +2268,15 @@
* @memberOf Benchmark.options
* @type Function
*/
'onStart': undefined
'onStart': undefined,

/**
* The locale used for formatting numbers
*
* @memberOf Benchmark.options
* @type string
*/
'locale': undefined
},

/**
Expand Down
Loading