-
-
Notifications
You must be signed in to change notification settings - Fork 16
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
IntlMessageFormat and custom formatters #5
Comments
hm...not sure if people add customFormatters per message or just add them like once as default to all messages... in i18next we have the https://www.i18next.com/translation-function/formatting we might just have some additional option added to icu init like: i18next
.use(ICU)
.init({
i18nFormat: {
localeData: fr, // you also can pass in array of localeData
formatters: {
upcase: function(v) { return v.toUpperCase(); },
locale: function(v, lc) { return lc; },
prop: function(v, lc, p) { return v[p] }
}
}
}); those get added to all compiled message functions here: https://github.com/i18next/i18next-icu/blob/master/src/index.js#L50 Sorry just take a quick look...not sure if i got it right how it should behave. |
From the user point of view the solution could look something like: const icu = new ICU();
icu.addLocaleData(fr);
icu.addFormatters({
upcase: function(v) { return v.toUpperCase(); },
locale: function(v, lc) { return lc; },
prop: function(v, lc, p) { return v[p] }
});
i18n.use(icu); |
we could provide both...an additional addFormatters function and passing them via options: Line 21 in 133d297
|
Yeah that'd be cool. I'll be happy to test this feature if you are fancy an experiment 😉 |
Will see when i find time...not hard to add this...more a problem of my time management 😭 |
No worries, I know your pain! Just ping me when you want to go for it and I'll find some time to give the feature a test 😉 |
hm...just started with it...as i thought simplest of all the shit i got planned....oh no...not really: https://github.com/yahoo/intl-messageformat/issues/121#issuecomment-176043615 IntlMessageFormat custom formatters are no custom formatters just some...idk what they are: https://github.com/yahoo/intl-messageformat#user-defined-formats And currently i do not expect them to do anything in this direction. So only option i currently see would be moving to messageformat |
😅 oh gosh that's confusing indeed! Looks like my brain mixed-up Although user-defined formats in var msg = new IntlMessageFormat('The price is: {price, number, USD}', 'en-US', {
number: {
USD: {
style : 'currency',
currency: 'USD'
}
}
}); This const icu = new ICU();
icu.addLocaleData(fr);
icu.addUserDefinedFormats({
number: {
THREE_FRACTIONAL_DIGITS: {
minimumSignificantDigits: 3,
maximumSignificantDigits: 3
},
ROUGH: {
maximumSignificantDigits: 1
},
VERY_PRECISE: {
minimumSignificantDigits: 6,
},
}
});
i18n.use(icu); And then: {
"example1": "Our value is equal to {value, number, THREE_FRACTIONAL_DIGITS}.",
"example2": "Our value is roughly equal to {value, number, ROUGH}, but if you like things to be precise, it is {value, number, VERY_PRECISE}.",
} There are plenty of options WDYT? |
hope tests are sufficient to get how it works: 9607f39 currently running travis...will npm publish if all green |
i18next-icu@0.5.0 |
Outstanding 🎉 I'll test your commit soon! Just one quick note before you've released this: should the method be called |
Ouch, looks like I was a bit late 😅 |
Just noticed that the third argument in It does not influence the behaviour in any way, just makes things a bit less obscure (especially given the fact that the things with IntlMessageFormat vs MessageFormat are confusing from start 😅 ). Let me give your new version a try first anyway! Given that it's unlikely that too many people will be use custom formats from day one, I'll be happy to see the methods renamed if you agree with me that this can positively influence the coherence of |
Custom formats work well when passed to the ICU constructor: const icu = new ICU({
formatters: {
number: {
THREE_FRACTION_DIGITS: {
minimumFractionDigits: 3,
maximumFractionDigits: 3,
},
},
},
}); This is truly great! 🎉 However, when I try using const icu = new ICU();
icu.addFormatters({
number: {
PRICE: {
minimumFractionDigits: 3,
},
},
}); I could get as far as adding several One thing worth noting in my most recent example is that I replaced |
changed test to use the add function -> works on my machine and travis... could you take another look - as i can't reproduce that. Only thing i know is having formats on init and additional adding via add function will overwrite them. also done renamings...hope you like that |
i18next-icu@0.6.0 includes changes. |
Great, thank you 🎉 👏 I'll try the changes tonight. |
Hi @jamuhl, just a quick heads up. I ran out of time this evening, so have to postpone my tests for another day. Hope this can be tomorrow – I'll let you know once I get there. Sorry for this. |
@kachkaev no problem...if there is something breaking it will be no big deal...i think after this i will change version to v1.0.0 anyway. |
I did some testing with this. The formatters passed in the ctor options works, but calling addUserDefinedFormats doesn't actually add the formats. |
@dyniper this worked for me to register a custom ICU format: import * as ICU from "i18next-icu";
const icu = new ICU({
formats: {
number: {
PRICE: {
minimumFractionDigits: 2,
useGrouping: false,
},
},
},
}); If this solution satisfies you too, let’s close this issue. |
Yes, this work, but if you do:
, that one doesn't work. I don't mind having to provide the formats in the ctor, but if there is a method to add them after constructing the object, it should work, otherwise, remove the method. |
@dyniper PR with a fix or removing that is welcome |
Will put that on the pile. Really busy these days and it makes hard to do community contributions. Thanks for everything you are doing. |
@dyniper sounds like a good plan 👍 |
First of all, thanks for creating such a fantastic tool and my apologies for commenting on such an old issue. However, I was trying to use the custom formatter like the code below and one of my translators appears to want a 24 hour time whereas for the rest of the locales I want 12 hour time. Is it possible to set these custom formatters based on the locale? Eg. if user is on en it would one set of configs and another locale another set. If this is not an existing feature, I am willing to contribute and open a PR for this if given some direction on what would be the best solution. Thanks! const icu = new ICU({
parseLngForICU: (lng) => {
return lng === 'en' ? 'en-GB' : lng;
},
memoize: true,
formats: {
number: {},
date: {
long: {
day: 'numeric',
month: 'short',
year: 'numeric',
weekday: 'long',
},
full: {
hour12: true,
day: 'numeric',
month: 'short',
year: 'numeric',
weekday: 'long',
hour: '2-digit',
minute: '2-digit',
},
},
time: {
short: {
hour12: true,
},
},
},
}); |
Hi @jamuhl 👋
I'm opening this issue to discuss how custom formatters could be added to
i18next-icu
. It seems thatMessageFormat
you are leveraging has this functionality already, it's jut not exposed into the API ofi18next-icu
. See the docs here:https://messageformat.github.io/messageformat/page-guide#toc11__anchor
https://messageformat.github.io/messageformat/MessageFormat.html#addFormatters
The reason why I have started exploring custom formatters is because I would like to output values with certain numbers of fractional digits, e.g. 1, 3 or 6.
Here's how my translation files could look in an ideal world:
I'm not yet sure what I'd write in
addFormatters()
to definenumberWithThreeFractionalDigits
,numberWithFD
or whatever. But before I get there, it'd be great to know howaddFormatters()
could be called fromi18next-icu
. I see you are doing something smart about memorisation ofIntlMessageFormat
here, which needs to be taken into account:i18next-icu/src/index.js
Line 24 in 133d297
i18next-icu/src/index.js
Lines 45 to 53 in 133d297
Being able to define custom formatters in the ICU plugin will make it as extensible as the main formatter. And in turn this will add even more value to the synergy i18next ecosystem creates!
The text was updated successfully, but these errors were encountered: