Skip to content

Commit

Permalink
Runtime: Fix unitFormatter with numberFormatter
Browse files Browse the repository at this point in the history
JSON.stringify omits functions, so the generated runtimeKey
did not depend on the value of the numberFormatter option,
causing different unitFormatters to have an identical runtimeKey.

Fixes globalizejs#704
  • Loading branch information
nkovacs committed May 22, 2017
1 parent 9389955 commit c85af7c
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 7 deletions.
5 changes: 3 additions & 2 deletions src/common/runtime-bind.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
define([
"./runtime-key",
"./runtime-stringify",
"../util/function-name"
], function( runtimeKey, functionName ) {
], function( runtimeKey, runtimeStringify, functionName ) {

return function( args, cldr, fn, runtimeArgs ) {

var argsStr = JSON.stringify( args ),
var argsStr = runtimeStringify( args ),
fnName = functionName( fn ),
locale = cldr.locale;

Expand Down
5 changes: 3 additions & 2 deletions src/common/runtime-key.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
define([
"./runtime-stringify",
"../util/string/hash"
], function( stringHash ) {
], function( runtimeStringify, stringHash ) {

return function( fnName, locale, args, argsStr ) {
var hash;
argsStr = argsStr || JSON.stringify( args );
argsStr = argsStr || runtimeStringify( args );
hash = stringHash( fnName + locale + argsStr );
return hash > 0 ? "a" + hash : "b" + Math.abs( hash );
};
Expand Down
12 changes: 12 additions & 0 deletions src/common/runtime-stringify.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
define([], function( ) {

return function( args ) {
return JSON.stringify( args, function( key, value ) {
if ( typeof value === "function" ) {
return value.runtimeKey; // if undefined, the value will be filtered out.
}
return value;
} );
};

});
5 changes: 2 additions & 3 deletions test/compiler/cases/unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,9 @@ module.exports = {
{ formatter: Globalize.unitFormatter( "hour", {
numberFormatter: Globalize.numberFormatter( { minimumIntegerDigits: 1 } )
} ), args: [ 3 ] },
// TODO: Fails because of https://github.com/globalizejs/globalize/issues/704
/* { formatter: Globalize.unitFormatter( "hour", {
{ formatter: Globalize.unitFormatter( "hour", {
numberFormatter: Globalize.numberFormatter( { minimumIntegerDigits: 2 } )
} ), args: [ 3 ] } */
} ), args: [ 3 ] },

{ formatter: en.unitFormatter( "day" ), args: [ 1 ] },
{ formatter: en.unitFormatter( "day" ), args: [ 100 ] },
Expand Down
6 changes: 6 additions & 0 deletions test/functional/unit/unit-formatter.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,4 +129,10 @@ QUnit.test( "should allow for runtime compilation", function( assert ) {
);
});

QUnit.test( "should generate different runtime key when using different numberFormatter", function( assert ) {
var formatter1 = Globalize.unitFormatter( "hour", { numberFormatter: Globalize.numberFormatter( { minimumIntegerDigits:1 } ) });
var formatter2 = Globalize.unitFormatter( "hour", { numberFormatter: Globalize.numberFormatter( { minimumIntegerDigits:2 } ) });
assert.notEqual( formatter1.runtimeKey, formatter2.runtimeKey );
});

});

0 comments on commit c85af7c

Please sign in to comment.