Skip to content
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

Currency in skeleton with unit-width-iso-code does not work. #62

Closed
omaraboualpha opened this issue Mar 30, 2023 · 9 comments
Closed

Currency in skeleton with unit-width-iso-code does not work. #62

omaraboualpha opened this issue Mar 30, 2023 · 9 comments

Comments

@omaraboualpha
Copy link

🐛 Bug Report

I'm trying to translate a string by passing in a numeric value, (auctionFee), but the skeleton implementation does not seem to return the correct formatting. It returns kr
{auctionFee, number, ::currency/SEK unit-width-iso-code}

To Reproduce

"i18next": "^21.10.0",
"i18next-http-backend": "^1.4.1",
"i18next-icu": "^2.1.0",
"i18next-vue": "^2.0.0-beta.0",
"intl-messageformat": "^10.3.3",
 "js-yaml": "^4.1.0",
 "vue": "^3.2.31",
// Paste your code here
JS (vue):
this.$t('JS_TEXT_LABEL_BID-PAYMENT-INFO_AUCTION-FEE_EX-VAT', {auctionFee: this.productData.auctionFee})
JS_TEXT_LABEL_BID-PAYMENT-INFO_AUCTION-FEE_EX-VAT: >-
  {auctionFee, number, ::currency/SEK unit-width-iso-code}

Expected behavior

I expect the skeleton to return the value sent in with SEK appended after the auctionFee value. What i'm getting back is "kr", and this is not the iso code for the currency. If i were to change "unit-width-iso-code" to "unit-width-full-name" I would get "svenska kronor" which is correct.

So when I'm expecting the ISO code it does not work.

Your Environment

  • i18next version: 21.10.0
  • os: Mac
@omaraboualpha omaraboualpha changed the title Currency in skeletion with unit-width-iso-code does not work. Currency in skeleton with unit-width-iso-code does not work. Mar 30, 2023
@jamuhl
Copy link
Member

jamuhl commented Mar 30, 2023

if it works in intl-messageformat there should be no reason it does not work here...as we basically just wrap that module for messageformat format calls

@omaraboualpha
Copy link
Author

omaraboualpha commented Mar 30, 2023

I guess I can't find anything about displaying the currency iso-code in the intl-messageformat docs. I was following the docs from here:
https://unicode-org.github.io/icu/userguide/format_parse/numbers/skeletons.html#unit-width

Do you know if there's a way displaying the currency iso-code using intl-messageformat?

@jamuhl
Copy link
Member

jamuhl commented Mar 30, 2023

Personally I only use i18next it's own format...so this really is nothing more than a wrapper for people prefer using message format based on that module of formatjs

@omaraboualpha
Copy link
Author

Ok, thank you for clarifying.

@omaraboualpha
Copy link
Author

After a lot of debugging, I found what I believe is a bug.

i18next-icu/i18nextICU.js

Lines 660 to 670 in cabd4c0

case 'unit-width-short':
result.currencyDisplay = 'code';
result.unitDisplay = 'short';
continue;
case 'unit-width-full-name':
result.currencyDisplay = 'name';
result.unitDisplay = 'long';
continue;
case 'unit-width-iso-code':
result.currencyDisplay = 'symbol';
continue;

case 'unit-width-short':
    result.currencyDisplay = 'code';
    result.unitDisplay = 'short';
    continue;
case 'unit-width-iso-code':
    result.currencyDisplay = 'symbol';
    continue;

It looks like the settings for these two options are swapped. According to mdn web docs, unit-width-iso-code should display a three letter ISO-code, which currently the option "unit-width-short" does, but "unit-width-iso-code" does not.

From my understanding "unit-width-short" should use the symbol option and "unit-width-iso-code" should use "code".

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat/NumberFormat#currencydisplay

If i use the IntlMessageFormat with code for currencyDisplay, the output result shows the ISO code of currency. So the i18next-icu wrapper seems wrong in this instance.

Example expected behaviour for unit-width-iso-code:

new IntlMessageFormat('The price is: {price, number, USD}', undefined, {
  number: {
    USD: {
      style: 'currency',
      currency: 'USD',
      currencyDisplay: 'code',
    }
  }
}).format({price: 100})

Output: The price is: USD 100.00

Example, expected behaviour for unit-width-short:

new IntlMessageFormat('The price is: {price, number, USD}', undefined, {
          number: {
            USD: {
              style: 'currency',
              currency: 'USD',
              currencyDisplay: 'symbol',
            }
          }
        }).format({price: 100})

Output: The price is: US$100.00

@jamuhl
Copy link
Member

jamuhl commented Mar 30, 2023

As your wrote before that you're using intl-messageformat": "^10.3.3 it must be an issue there - as our code does not really contain that code you pasted -> that is just bundled from intl-messageformat

But you're directly using latest 10.3.3 of that module - so the issue must still be included there.

@omaraboualpha
Copy link
Author

To clarify, when i used "new IntlMessageFormat", it was to show that it works when i use new IntlMessageFormat directly, but when i try to translate with icu it doesnt work, and that's because of the rules in the switch case i linked to that are from the file: i18next-icu/i18nextICU.js

But this issue seems to be related to the package "icu-skeleton-parser". There has been issues raised for this in the formatjs repo:
formatjs/formatjs#4038 (comment)

@jamuhl
Copy link
Member

jamuhl commented Mar 31, 2023

what I said ... is if you're using i18next-icu in not "stone-age" way of require js -> which I assume as you pasted your package.json versions....you don't use i18nextICU.js at all -> you use the npm package es version in the dist folder and whatever version of intl-messageformat, which itself uses that icu-skeleton-parser

in case you really rely on that i18nextICU.js (which looks odd to me) I just updated local intl-messageformat to 10.3.3 and published 18next-icu@2.2.0 (hope that solves your issue)

@omaraboualpha
Copy link
Author

You are right in that I'm using the es version, by importing the i18next-icu and bundling through webpack. I highlighted the code in that file to highlight where i noticed the issue with the code, and as you said, the issue probably resides in the icu-skeleton-parser. I tried updating but still the same issue. I can use unit-width-short for now to resolve my issues locally, and update these to the right styles when icu-skeleton-parser fixes the issue.

So the issue is not in this plugin, but since you include it, it's good to know so that when they resolve the issue, you also update that dependency, if this is automated i don't know. But the issue in this repo can be closed since we narrowed it down to icu-skeleton-parser.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants