-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ICU 68.1 upgrade changes currency formatting output in Node 14.17 #38897
Comments
Updates of CLDR data probably always result in the change of the output in some cases. I don't think it was ever discussed if these would qualify as semver-major. They can be seen as bug fixes too. FWIW, you can get the previous behavior consistently with: console.log(123.45.toLocaleString('fr-CA', { style: 'currency', currency: 'CAD', currencyDisplay: 'narrowSymbol' })); |
/cc @nodejs/i18n-api |
Yes, I would be surprised if there wasn't a change in output when new CLDR data comes in . You can check the CLDR version with @iansu :
I would like to hear more about the use case here. Specifically, how this would produce a production issue.. It is not recommended to have tests which explicitly verify formatted output, it's for humans to see. For example, for network transit between software, disk storage, etc. a binary or locale-neutral format ought to be used. I'd be happy to help comment on any design approach if I can help. |
@obensource a good example of a recent CLDR change @nodejs/i18n-api what is the right label for "locale data changed due to CLDR"? wontfix? |
@targos Thank you for suggesting that fix. That does seem to resolve the issue. I can see how this is expected to happen but it would be great if this was somehow more clear. This was a surprising change to me and it also took quite a while for me to figure out what was going on, where to report it, if I should report it at all, etc. I don't have any concrete solutions for this but I'm active in this project and I was confused so this would probably be confusing to others as well.
I don't mean production issue as in a crash or an outage. I'm using this to generate financial documents and in some cases there are rules around how things need to be displayed. This would have changed those documents and, given the additional characters in the new format, could have caused other formatting issues. |
That is specifically requesting a different display, but is definitely not guaranteed to be identical.
I certainly would like to improve anywhere that we can.
It's certainly fine to report it here. ( @obensource this is where we need to make the linkage node -> v8 -> ICU -> CLDR more clear. )
If there are rules about how things are to be displayed, locale sensitive output may not be the right choice for you, because you would need to implement those rules. If the rules say, "ASCII digits, followed by dollarsign" … etc. The purpose of the locale strings has always been to attempt to output something that will be readable to people with their own language and cultural preferences. Think of a general user viewing some financial information. As I mentioned, if your documents have a specific requirement, they fall somewhat outside of this. Ideally you would use an ( @sffc do you know of any ecma402 proposals that allow specifying one's own pattern and symbol information, as with a locale-less ICU formatter?) |
The default currency symbol is the CLDR short symbol, which is intended as a symbol which is unambiguously understood in that locale. You can see that this symbol was changed in CLDR 38 to add the "CA" to the end: https://unicode-org.github.io/cldr-staging/charts/38/delta/fr.html#Northern%20America:%20Canada The suggestion above to use
Not at this time. If someone requires a very specific output format, then Intl is probably not what they are looking for. The string outputs from Intl are locale-dependent and could change over time. However, something that could be in scope for ECMA-402 would be |
@iansu I think the remaining issues in this issue are in two categories:
Any ideas? Where did/would you look? |
While working on a feature, we found that our unit tests were failing during CI but not locally, and the same test was passing yesterday in CI. The failing test was being caused by `toLocaleString()` now returning `Sept` instead of `Sep` for September, which breaks date formatting for transaction and customer files. On investigation we found that the version of Node being used by Docker was previously 16.13.2 but was now 16.4.0. It appears that the underlying internationalisation data that node uses has changed between these versions. As stated in a [previous related error](nodejs/node#38897): > The purpose of the locale strings has always been to attempt to output something that will be readable to people with their own language and cultural preferences. Think of a general user viewing some financial information. As I mentioned, if your documents have a specific requirement, they fall somewhat outside of this. Since the output of `toLocaleString()` cannot be relied on to be consistent, we ditch it completely and instead construct the date string ourselves.
Closing as not an issue… |
What steps will reproduce the bug?
Running this code in Node 14.16 and 14.17 produces different results:
In Node 14.16 the output is
123,45 $
In Node 14.17 the output is
123,45 $ CA
How often does it reproduce? Is there a required condition?
Always
What is the expected behavior?
There should be no change in output
What do you see instead?
Different output between minor Node versions (14.16 and 14.17)
Additional information
I'm not certain if this is an issue with Node or the ICU data library. All I know is this output has changed in a minor version release which is unexpected to me and maybe seems like a breaking change. This caused my tests to fail and likely would have caused a production issue if my tests hand't caught it.
The ICU data was upgraded to version 68.1 in Node 14.17 here: #36187
The text was updated successfully, but these errors were encountered: