search

globalizejs / globalize

A JavaScript library for internationalization and localization that leverages the official Unicode CLDR JSON data
https://globalizejs.com
MIT License
4.8k stars 603 forks source link

Runtime: Fix unitFormatter with numberFormatter #719

Closed nkovacs closed 6 years ago

nkovacs commented 7 years ago

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 #704

rxaviers commented 7 years ago

Thanks @nkovacs!! Great catch!

I left a comment above. What do you think? Just let me know if you have any questions about it...

nkovacs commented 7 years ago

Pull request for compiler is up: https://github.com/globalizejs/globalize-compiler/pull/31 and I've removed the hack in core-runtime.js.

nkovacs commented 7 years ago

I've rebased on master and uncommented the compiler test. Travis fails because of the missing compiler change.

rxaviers commented 7 years ago

Ok thanks

rxaviers commented 7 years ago

@nkovacs, other formatters (e.g., date, relative time) also include a number formatter in their runtime properties. We need to understand why this happens with unit formatter, but not with the other ones...

nkovacs commented 7 years ago

No other formatter accepts a formatter as an option which should affect the runtimeKey.

rxaviers commented 7 years ago

True! Thanks

rxaviers commented 6 years ago

Thanks @nkovacs!

Pull request for compiler is up: globalizejs/globalize-compiler#31 and I've removed the hack in core-runtime.js.

Instead of depending on globalize-compiler to include the runtimeKey info, I've update the runtime function to do it.