Intl.NumberFormat notation: compact is not working on 0.71.7
Bug Description
- [x] I have run
gradle cleanand confirmed this bug does not occur with JSC
Hermes version: { "Bytecode Version": 90, "Builtins Frozen": false, "VM Experiments": 0, "Build": "Release", "GC": "hades (concurrent)", "OSS Release Version": "for RN 0.71.7", "CommonJS Modules": "None" }
React Native version (if any): 0.71.7" OS version (if any): 0.71.7" Platform (most likely one of arm64-v8a, armeabi-v7a, x86, x86_64): all
Steps To Reproduce
- Use Intl.NumberFormat with large number and notation as compact
code example:
function formatCompactNumber(number) {
const formatter = Intl.NumberFormat("en", { notation: "compact" });
console.log(formatter.format(number));
}
// the comments are what should be logged
formatCompactNumber(-57); // -57
formatCompactNumber(999); // 999
formatCompactNumber(8_554); // 8.5K
formatCompactNumber(150_000); // 150K
formatCompactNumber(3_237_512); // 3.2M
formatCompactNumber(9_782_716_897); // 9.8B
formatCompactNumber(7_899_693_036_970); // 7.9T
// below is what we got:
/*
LOG -57
LOG 999
LOG 8,554
LOG 150,000
LOG 3,237,512
LOG 9,782,716,897
LOG 7,899,693,036,970
*/
The Expected Behavior
Expected:
formatCompactNumber(150_000); // 150K
formatCompactNumber(3_237_512); // 3.2M
formatCompactNumber(9_782_716_897); // 9.8B
formatCompactNumber(7_899_693_036_970); // 7.9T
Hey @AndreiCalazans thank you for reporting this.
Are you observing this on iOS or Android? In general, our Intl APIs are implemented on top of APIs exposed by the underlying platform, so there are gaps where we cannot map cleanly onto the underlying platform.
For android it is documented on the website that using the compact option has some "rough edges". It says nothing about iOS tough. Saying it has "rough edges" cloud maybe be more specific as it this case it seems to mean not making it compact at all.
Edit: Just ran this on MacOS (without bundeling with react native) and can confirm in am getting those same 'non compact' results. I am not very familiar with the codebase yet so is it ok to just run hermes standalone and try to work on a fix or should changes to Intl always be run and tested in a RN app? Asking this because as you said they are using platform apis.
@jobpaardekooper Thanks for trying it out. Building the CLI on macOS should be good enough for developing for macOS and iOS, since the APIs available are mostly the same. The only thing to be careful about is that we can't use ICU, because access to the system ICU may be allowed on macOS, but is blocked on iOS.
For Android, the only way currently is to build an app, since the implementation is in Java and uses Android APIs.
We'd welcome discussions of potential implementations, and a PR. As a starting point, you can take a look at PlatformIntlApple.mm for macOS development, since that should be the most convenient to start trying things out.
Thank you for pointing me in the right direction! Will start looking into the code a bit more and will try to make a fix.
From what I could find there are no tests for this specific functionality. Should that be added or is it fine like it is? If it should be added do you maybe have some tips/resources to get started for writing the tests? Or a document/file where I can find info about the testing for Intl.
Also another question that kind of confused me. When running hermes --help the following option is listed:
-Xintl - Enable support for ECMA-402 Intl API
But Intl only seems to be working when compiled in with the DHERMES_ENABLE_INTL option set. All works fine when it has been compiled in and you run without -Xintl. What am I misunderstanding about this flag?
Again, thanks a lot for the help.
Regarding testing, we currently run against test262, which is the primary source of tests for both Android and iOS Intl implementations. You can take a look at our CI set-up here to see how it is done.
We also have some additional tests here that you can add to if you add features. Note that these are primarily for running on macOS.
Intl is enabled by default when you build with it, so you shouldn't need to touch -Xintl unless you want to disable it.
@neildhar I was looking into it a little bit and found the following comment in PlatformIntlApple.mm:
// NOTE: NSNumberFormatter has following limitations:
// - "scientific" notation is supprted, "engineering" and "compact" are not.
// - roundingType is not supported.
// - compactDisplay is not supported.
// - signDisplay is not supported.
// - NSNumberFormatter has maximumIntegerDigits, which is 42 by default
All this iOS stuff is implemented using Apple's NSNumberFormatter. But there is no NSNumberFormatter that supports these cases listed in the comment above so I am assuming that is why they have not been implemented. I don't think this would be easy to do seeing as we can't use ICU and Apple does not have support for it. Maybe you have any ideas on how this could be achieved?
Additionally, the "Internationalization APIs" page in the docs does not mention that these options are not supported. It actually specifically mentions that the format function is supported according to the standard and then continues to list a few limitations for android. I think these iOS limitations should also be added to the docs just like the android limitations are there.
Do you have any idea how this could feasibly be implemented? Because this example of using 'compact' varies heavily for each language. If not I can create a PR to add this information to the docs.
any ideas on how this could be achieved
I do not unfortunately. I remember looking into these when we first rolled out NumberFormatter support on iOS, and the lack of support for compact in Foundation seemed to be a known limitation.
I can create a PR to add this information to the docs
We would welcome such a PR. As you have observed, the page currently only talks about Android, and we should document the iOS support as well.
@neildhar I have created the PR. Maybe you can review it?
Hey, I was away for the past weeks. I caught this issue on iOS and I did not test on Android. @jobpaardekooper I see that you started documenting the limitations, did you test compact works on Android too?
ps: thanks @jobpaardekooper for investigating this :heart:
@AndreiCalazans I did not test it but in the docs at the Android 11 section it says notation: 'compact' has some rough edges on Android.
@neildhar As we had discussed a while ago, NumberFormatter does not allow for this "compact" functionality. Now I see that FormatStyle actually supports compact notation. For example:
Float(9_782_716_897).formatted(.number.notation(.compactName)) // 9,8B
There are two major issues with this though.
- This is only available since iOS 15.
- This is a Swift only API.
I can imagine there might be a work around possible regarding the Swift only thing. But, I think iOS 15+ is a complete deal breaker right? Or would there be some way to optionally enable this?
is there a workaround for this? doesn't work on android and ios for me
@jobpaardekooper We could conditionally compile it out on older iOS versions, but this wouldn't help in general until RN's minimum iOS version is past 15+. (except for users who are willing to build Hermes from source)
Unfortunately for us we were forced to polyfill with @formatjs
Unfortunately for us we were forced to polyfill with
@formatjs
We did as well... but it is insanely slow (I use the formatter on a list with a lot of elements). It would be very nice to see a native implementation both on iOS and Android. maybe it can be polyfilled on the native side?
@jgo80 double check that formatjs ins't using Hermes' native Intl APIs since they are available formatjs won't oveerride them. We had to patch shouldPolyfill calls within formatjs to always return true.
Hermes' native Intl APIs are very slow.
@AndreiCalazans Thx. Even with import '@formatjs/intl-numberformat/polyfill-force' (and forcing all of the other @formatjs imports) it remains slower than without polyfill. It has significant impact on the JS Thread. Still hoping for native support 🤞
FYI, this is our plan for dealing with Intl problems: https://github.com/facebook/hermes/discussions/1211
Is there a workaround for this?
Ok as a workaround you can use this function:
function customCompactFormat(number) {
if (number < 1e3) {
return number.toString();
} else if (number < 1e6) {
return Math.floor(number / 1e2) / 10 + 'K'; // Formats thousands
} else if (number < 1e9) {
return Math.floor(number / 1e5) / 10 + 'M'; // Formats millions
} else {
return Math.floor(number / 1e8) / 10 + 'B'; // Formats billions
}
}
I made this function to convert my numbers, to continue using Hermes.
function customIntl(
number = 0,
style = "currency",
locale = "en-US",
currency = "USD"
) {
const absNumber = Math.abs(number);
const units = ["", "K", "M", "B", "T"];
let unitIndex = 0;
let num = absNumber;
while (num >= 1000 && unitIndex < units.length - 1) {
num /= 1000;
unitIndex++;
}
const formattedNumber = num.toLocaleString(locale, {
minimumFractionDigits: 0,
maximumFractionDigits: 2,
});
const formattedCurrency = new Intl.NumberFormat(locale, {
style,
currency: currency,
minimumFractionDigits: 0,
maximumFractionDigits: 2,
}).format(Number(formattedNumber));
return `${formattedCurrency}${units[unitIndex]}`;
}