Support BigInt in Intl.NumberFormat.format() and formatToParts()#1930
Support BigInt in Intl.NumberFormat.format() and formatToParts()#1930tmikov wants to merge 1 commit intofacebook:static_hfrom
Conversation
Per ECMA-402 §15.5.4, BigInt is a valid argument to NumberFormat format and formatToParts methods. Previously these called toNumber_RJS() which throws TypeError on BigInt inputs. Replace toNumber_RJS() with toNumeric_RJS() and, when the result is a BigInt, convert it to a decimal string via bigint::toString() and pass it through new string-based format/formatToParts overloads on each platform backend (ICU stub, Apple via NSDecimalNumber, Android via java.math.BigDecimal). This preserves exact precision for arbitrarily large BigInts. Fixes facebook#1928
meta-cla
bot
added
the
CLA Signed
There was a problem hiding this comment.
Pull request overview
Adds ECMA-402–compliant BigInt handling to Intl.NumberFormat.prototype.format() and formatToParts() by routing BigInt inputs through string-based formatting paths in the platform Intl backends.
Changes:
- Switch
Intl.NumberFormatargument coercion fromtoNumber_RJS()totoNumeric_RJS()and add BigInt-specific formatting branches. - Introduce string overloads for
format/formatToPartsacross the platform Intl layer (C++/JNI + Java backends). - Expand the regression test to validate formatted output for several BigInt values.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/hermes/regress-intl-number-format-bigint-input.js | Updates regression test to assert concrete formatted outputs for BigInt format() calls. |
| lib/VM/JSLib/Intl.cpp | Accepts BigInt via toNumeric_RJS() and formats BigInt by converting to a base-10 string. |
| lib/Platform/Intl/java/com/facebook/hermes/intl/PlatformNumberFormatterICU.java | Adds format(String) / formatToParts(String) using BigDecimal for ICU-backed Android formatting. |
| lib/Platform/Intl/java/com/facebook/hermes/intl/PlatformNumberFormatterAndroid.java | Adds format(String) / formatToParts(String) using BigDecimal for the legacy Android formatter. |
| lib/Platform/Intl/java/com/facebook/hermes/intl/NumberFormat.java | Adds JNI-exposed format(String) / formatToParts(String) overloads that delegate to the platform formatter. |
| lib/Platform/Intl/java/com/facebook/hermes/intl/IPlatformNumberFormatter.java | Extends the platform formatter interface with string overloads for BigInt formatting. |
| lib/Platform/Intl/PlatformIntlICU.cpp | Adds string overload stubs for ICU (non-Android/Apple) NumberFormat formatting/parts. |
| lib/Platform/Intl/PlatformIntlApple.mm | Adds string overload for Apple NumberFormat using NSDecimalNumber (and wires through base class). |
| lib/Platform/Intl/PlatformIntlAndroid.cpp | Adds JNI bindings to call the new Java format(String) / formatToParts(String) overloads. |
| include/hermes/Platform/Intl/PlatformIntl.h | Extends C++ platform Intl NumberFormat API with string overloads. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Ensures Hermes Intl's NumberFormat correctly formats BigInt inputs. | ||
| var nf = new Intl.NumberFormat("en", undefined); | ||
|
|
There was a problem hiding this comment.
This regression test now validates BigInt handling for format(), but the PR also changes formatToParts() to accept BigInt. Adding at least one formatToParts(…n) assertion (including a negative BigInt case) would help prevent regressions in the new BigInt-specific formatToParts path.
| if (keyIterator.hasNext()) { | ||
| key = mPlatformNumberFormatter.fieldToString(keyIterator.next(), 0); | ||
| } else { | ||
| key = "literal"; | ||
| } |
There was a problem hiding this comment.
In the BigInt string overload of formatToParts(), fieldToString() is always called with 0, which makes the SIGN field mapping incorrect for negative BigInt values (it will be reported as plusSign instead of minusSign in the ICU backend). Consider deriving the sign from n (e.g., leading '-') and passing a negative value when needed, or adding a dedicated fieldToString overload for non-double inputs so sign classification stays correct without lossy numeric conversion.
| BigDecimal bigDecimal = new BigDecimal(n); | ||
| try { |
There was a problem hiding this comment.
BigDecimal bigDecimal = new BigDecimal(n); is executed before the try/catch, so an invalid decimal string would throw NumberFormatException that bypasses your fallback handling. Move the BigDecimal construction into the try block (or add an outer catch) so the method’s error handling is actually effective.
| BigDecimal bigDecimal = new BigDecimal(n); | |
| try { | |
| BigDecimal bigDecimal = null; | |
| try { | |
| bigDecimal = new BigDecimal(n); |
Per ECMA-402 §15.5.4, BigInt is a valid argument to NumberFormat format and formatToParts methods. Previously these called toNumber_RJS() which throws TypeError on BigInt inputs.
Replace toNumber_RJS() with toNumeric_RJS() and, when the result is a BigInt, convert it to a decimal string via bigint::toString() and pass it through new string-based format/formatToParts overloads on each platform backend (ICU stub, Apple via NSDecimalNumber, Android via java.math.BigDecimal). This preserves exact precision for arbitrarily large BigInts.
Fixes #1928