fix(amount): added bigInt and hideCurrency support in amount component#793
fix(amount): added bigInt and hideCurrency support in amount component#793
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR extends the Amount component with two complementary features. First, it broadens the Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/raystack/components/amount/__tests__/amount.test.tsx`:
- Around line 251-271: The hideCurrency test suite only covers non-round
amounts; add tests that assert behavior for round amounts (e.g., render <Amount
value={1200} hideCurrency /> and expect "12.00") and for bigint round values
(e.g., value={1200n} hideCurrency expecting "1,200") to ensure fraction digits
are preserved, and duplicate the override and zero-decimal (JPY) cases with
round values (e.g., currencyDisplay='code' and currency='JPY' with value=1200)
so the suite catches the fraction-digit regression in the Amount component.
In `@packages/raystack/components/amount/amount.tsx`:
- Around line 223-234: The current formatOptions object (in Amount component)
leaves minimumFractionDigits/maximumFractionDigits undefined when hideCurrency
is true, causing decimal digits to be stripped for round values; update the
hideCurrency branch inside formatOptions so it sets style: 'decimal' and also
explicitly applies the currency-driven fraction digits (use
minimumFractionDigits and maximumFractionDigits or the computed decimals value
used elsewhere) so decimals are preserved when hideCurrency=true; adjust tests
for bigint expectations and add a new test case asserting round amounts (e.g.,
value=1200) still render the expected trailing zeros.
- Around line 239-240: The code calls Intl.NumberFormat.format(finalBaseValue)
with finalBaseValue as a string to preserve precision, but environments may not
support V3 string-precision; either ensure your supported browsers include V3
(update browserslist/README) or add a runtime feature-detection and fallback:
implement the suggested supportsStringFormat check (new
Intl.NumberFormat('und').format('11111111111111111112').indexOf('2') !== -1)
and, if false, convert finalBaseValue to a safe fallback (e.g.,
Number(finalBaseValue) or a BigInt-to-string grouping formatter) before calling
Intl.NumberFormat.format; update the code around the
Intl.NumberFormat.format(...) call in amount.tsx that uses finalBaseValue
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9bb66c33-bcb5-4f9f-8b88-800f3a0c6b4e
📒 Files selected for processing (5)
apps/www/src/content/docs/components/amount/demo.tsapps/www/src/content/docs/components/amount/index.mdxapps/www/src/content/docs/components/amount/props.tspackages/raystack/components/amount/__tests__/amount.test.tsxpackages/raystack/components/amount/amount.tsx
| describe('hideCurrency', () => { | ||
| it('hides the currency symbol while preserving formatting', () => { | ||
| render(<Amount value={1299} hideCurrency />); | ||
| expect(screen.getByText('12.99')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('hides currency for bigint values', () => { | ||
| render(<Amount value={1299n} hideCurrency />); | ||
| expect(screen.getByText('1,299')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('overrides currencyDisplay when set', () => { | ||
| render(<Amount value={1299} hideCurrency currencyDisplay='code' />); | ||
| expect(screen.getByText('12.99')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('respects the currency for decimal-place math even when hidden', () => { | ||
| render(<Amount value={1299} currency='JPY' hideCurrency />); | ||
| expect(screen.getByText('1,299')).toBeInTheDocument(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
hideCurrency test suite covers only non-round amounts
All four hideCurrency tests use value={1299} (USD → 12.99) or a zero-decimal currency (JPY). None exercise a round amount like value={1200} (USD → 12.00) where the fraction digit discrepancy from the major issue above would manifest. The coverage gap means the bug is undetected by the test suite even though the docs promise "fraction digits are preserved."
These tests will need to be updated once the fix proposed in the amount.tsx comment is applied.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/raystack/components/amount/__tests__/amount.test.tsx` around lines
251 - 271, The hideCurrency test suite only covers non-round amounts; add tests
that assert behavior for round amounts (e.g., render <Amount value={1200}
hideCurrency /> and expect "12.00") and for bigint round values (e.g.,
value={1200n} hideCurrency expecting "1,200") to ensure fraction digits are
preserved, and duplicate the override and zero-decimal (JPY) cases with round
values (e.g., currencyDisplay='code' and currency='JPY' with value=1200) so the
suite catches the fraction-digit regression in the Amount component.
| const formatOptions: Intl.NumberFormatOptions = { | ||
| minimumFractionDigits: hideDecimals ? 0 : minimumFractionDigits, | ||
| maximumFractionDigits: hideDecimals ? 0 : maximumFractionDigits, | ||
| useGrouping: groupDigits | ||
| useGrouping: groupDigits, | ||
| ...(hideCurrency | ||
| ? { style: 'decimal' } | ||
| : { | ||
| style: 'currency', | ||
| currency: validCurrency.toUpperCase(), | ||
| currencyDisplay | ||
| }) | ||
| }; |
There was a problem hiding this comment.
hideCurrency=true does not preserve currency-driven fraction digits
The spread { style: 'decimal' } for hideCurrency doesn't override minimumFractionDigits, so it inherits undefined from the base object. For style: 'decimal', the default minimumFractionDigits is 0, whereas the default maximum digit value is 3 for plain values, 2 for currency. This means for round amounts, trailing zeros are stripped: value={1200} (USD minor units) formats to "12" instead of "12.00", directly contradicting the JSDoc comment on Line 77 ("fraction digits are preserved") and the PR summary ("fraction-digits remain").
The test suite covers only value={1299} (which produces non-zero decimal digits 12.99), so this discrepancy is invisible in tests. The same issue affects bigint values: 1299n with hideCurrency=false gives "$1,299.00" but hideCurrency=true gives "1,299".
🛠️ Proposed fix — propagate `decimals` as default in the `hideCurrency` branch
- const formatOptions: Intl.NumberFormatOptions = {
- minimumFractionDigits: hideDecimals ? 0 : minimumFractionDigits,
- maximumFractionDigits: hideDecimals ? 0 : maximumFractionDigits,
- useGrouping: groupDigits,
- ...(hideCurrency
- ? { style: 'decimal' }
- : {
- style: 'currency',
- currency: validCurrency.toUpperCase(),
- currencyDisplay
- })
- };
+ const formatOptions: Intl.NumberFormatOptions = {
+ useGrouping: groupDigits,
+ ...(hideCurrency
+ ? {
+ style: 'decimal',
+ minimumFractionDigits: hideDecimals ? 0 : (minimumFractionDigits ?? decimals),
+ maximumFractionDigits: hideDecimals ? 0 : (maximumFractionDigits ?? decimals),
+ }
+ : {
+ style: 'currency',
+ currency: validCurrency.toUpperCase(),
+ currencyDisplay,
+ minimumFractionDigits: hideDecimals ? 0 : minimumFractionDigits,
+ maximumFractionDigits: hideDecimals ? 0 : maximumFractionDigits,
+ })
+ };After this fix, also update the bigint hideCurrency test expectation:
- expect(screen.getByText('1,299')).toBeInTheDocument();
+ expect(screen.getByText('1,299.00')).toBeInTheDocument();And add a coverage case for round amounts:
it('preserves currency fraction digits for round amounts', () => {
render(<Amount value={1200} hideCurrency />);
expect(screen.getByText('12.00')).toBeInTheDocument();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const formatOptions: Intl.NumberFormatOptions = { | |
| minimumFractionDigits: hideDecimals ? 0 : minimumFractionDigits, | |
| maximumFractionDigits: hideDecimals ? 0 : maximumFractionDigits, | |
| useGrouping: groupDigits | |
| useGrouping: groupDigits, | |
| ...(hideCurrency | |
| ? { style: 'decimal' } | |
| : { | |
| style: 'currency', | |
| currency: validCurrency.toUpperCase(), | |
| currencyDisplay | |
| }) | |
| }; | |
| const formatOptions: Intl.NumberFormatOptions = { | |
| useGrouping: groupDigits, | |
| ...(hideCurrency | |
| ? { | |
| style: 'decimal', | |
| minimumFractionDigits: hideDecimals ? 0 : (minimumFractionDigits ?? decimals), | |
| maximumFractionDigits: hideDecimals ? 0 : (maximumFractionDigits ?? decimals), | |
| } | |
| : { | |
| style: 'currency', | |
| currency: validCurrency.toUpperCase(), | |
| currencyDisplay, | |
| minimumFractionDigits: hideDecimals ? 0 : minimumFractionDigits, | |
| maximumFractionDigits: hideDecimals ? 0 : maximumFractionDigits, | |
| }) | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/raystack/components/amount/amount.tsx` around lines 223 - 234, The
current formatOptions object (in Amount component) leaves
minimumFractionDigits/maximumFractionDigits undefined when hideCurrency is true,
causing decimal digits to be stripped for round values; update the hideCurrency
branch inside formatOptions so it sets style: 'decimal' and also explicitly
applies the currency-driven fraction digits (use minimumFractionDigits and
maximumFractionDigits or the computed decimals value used elsewhere) so decimals
are preserved when hideCurrency=true; adjust tests for bigint expectations and
add a new test case asserting round amounts (e.g., value=1200) still render the
expected trailing zeros.
| // @ts-expect-error TS lib types omit `string` from format() params, but Intl.NumberFormat accepts numeric strings at runtime — needed for large values that would lose precision as `number`. | ||
| } as Intl.NumberFormatOptions).format(finalBaseValue); | ||
| ).format(finalBaseValue); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/raystack/components/amount/amount.tsx | sed -n '230,250p'Repository: raystack/apsara
Length of output: 860
🏁 Script executed:
find . -name "package.json" -type f | head -5Repository: raystack/apsara
Length of output: 206
🏁 Script executed:
cat package.json | jq '.browserslist, .engines, .targets' 2>/dev/null || cat package.json | grep -A 5 -E '(browserslist|engines|targets)'Repository: raystack/apsara
Length of output: 90
🏁 Script executed:
cat packages/raystack/package.json | jq '.browserslist, .engines, .targets, .exports, .main' 2>/dev/null || cat packages/raystack/package.jsonRepository: raystack/apsara
Length of output: 691
🌐 Web query:
Intl.NumberFormat V3 browser support timeline string precision ES2023
💡 Result:
Intl.NumberFormat V3 refers to the ECMA-402 proposal adding advanced features like roundingPriority ("auto", "morePrecision", "lessPrecision"), roundingMode, and enhanced string precision control via minimumSignificantDigits/maximumSignificantDigits (1-21 range) in conjunction with fractionDigits, resolving conflicts via roundingPriority. These enable precise string-based number formatting without floating-point loss, key for ES2023 Intl updates.[1][2][3][4] Browser support timeline (full V3 features like roundingMode, roundingPriority): - Chrome/Edge: 106+ (Oct 2022)[1][5][6] - Firefox: 93 (initial), full 116+ (2023)[1][6] - Safari: 15.4+ (Mar 2022)[1][6] By 2026-05-07, universally supported in modern browsers (global usage >98%). Basic significantDigits supported earlier (Chrome 24+, Firefox 29+, Safari 10+).[7] Example for string precision: new Intl.NumberFormat('en-US', { maximumSignificantDigits: 3, roundingPriority: 'lessPrecision' }).format('1234.56789'); // "1,230" (respects both sig/maxFraction via priority)[3][4] Official spec in ECMAScript 2023 Intl API (10.0).[8]
Citations:
- 1: https://github.com/tc39/proposal-intl-numberformat-v3
- 2: https://tc39.es/proposal-intl-numberformat-v3/out/numberformat/proposed.html
- 3: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/NumberFormat/NumberFormat
- 4: https://stackoverflow.com/questions/55834596/use-intl-numberformat-with-maximumfractiondigits-and-maximumsignificantdigi
- 5: javascript.builtins.Intl.NumberFormat - The supporting version chart is out of date mdn/browser-compat-data#20258
- 6: https://caniuse.com/mdn-javascript_builtins_intl_numberformat_numberformat_options_parameter_options_roundingmode_parameter
- 7: https://caniuse.com/mdn-javascript_builtins_intl_numberformat_numberformat_options_parameter_options_minimumsignificantdigits_parameter
- 8: https://402.ecma-international.org/10.0/index.html
🏁 Script executed:
grep -r "Amount" packages/raystack/package.json | head -10Repository: raystack/apsara
Length of output: 41
🏁 Script executed:
grep -B5 "finalBaseValue" packages/raystack/components/amount/amount.tsx | head -20Repository: raystack/apsara
Length of output: 826
🏁 Script executed:
cat packages/raystack/README.md 2>/dev/null | head -50 || echo "No README found"Repository: raystack/apsara
Length of output: 1785
🏁 Script executed:
find . -maxdepth 2 -name ".browserslistrc" -o -name "tsconfig.json" -o -name "tsconfig.build.json" -o -name "vite.config.ts" -o -name "tsup.config.ts" 2>/dev/null | head -10Repository: raystack/apsara
Length of output: 41
🏁 Script executed:
cat tsconfig.json | jq '.compilerOptions.lib, .compilerOptions.target, .compilerOptions.useDefineForClassFields' 2>/dev/null || grep -A 5 '"target"' tsconfig.jsonRepository: raystack/apsara
Length of output: 105
🏁 Script executed:
find packages/raystack -name "tsconfig.json" -o -name "vite.config.ts" -o -name "tsup.config.ts" 2>/dev/nullRepository: raystack/apsara
Length of output: 91
🏁 Script executed:
find . -maxdepth 3 -name ".browserslistrc" 2>/dev/nullRepository: raystack/apsara
Length of output: 41
🏁 Script executed:
cat packages/raystack/tsconfig.json 2>/dev/null || echo "Not found"Repository: raystack/apsara
Length of output: 568
🏁 Script executed:
cat packages/tools-config/tsconfig/react.json 2>/dev/null || find . -path "*tools-config*" -name "*.json" -type f | head -10Repository: raystack/apsara
Length of output: 310
🏁 Script executed:
find . -maxdepth 2 -name "*.config.ts" -o -name "*.config.js" -type f | grep -E "(vite|tsup|esbuild|webpack)" | head -5Repository: raystack/apsara
Length of output: 41
🏁 Script executed:
cat packages/raystack/package.json | jq '.browserslist' 2>/dev/nullRepository: raystack/apsara
Length of output: 64
🏁 Script executed:
cat packages/tools-config/tsconfig/base.json 2>/dev/null | head -30Repository: raystack/apsara
Length of output: 580
🌐 Web query:
Intl.NumberFormat string parameter support es5 es6 when added specification history
💡 Result:
Intl.NumberFormat supports string parameters for the locales argument (a BCP 47 language tag string or array of strings) and accepts an options object as the second parameter. This API is not part of ES5 (ECMA-262 5th edition, 2009), which has no Intl object or NumberFormat.[1][2] It was first specified in ECMA-402 1st edition (December 2012), the ECMAScript Internationalization API Specification, with the constructor defined as Intl.NumberFormat([locales [, options ]]), where locales can be a string.[3][4][5][6] ECMA-402 1st edition aligned with ES6 (ECMA-262 6th edition, June 2015) development timeline, but the core API (including string locales support) was present from the initial 2012 spec and unchanged in basic constructor signature across editions.[7][8][9] Browser support began pre-ES6: Chrome 24 (2013), Firefox 29 (2014), Safari 10 (2016).[10][11] No evidence of string parameter support in ES5; Intl was added via ECMA-402.[12][13]
Citations:
- 1: https://402.ecma-international.org/10.0/index.html
- 2: https://tc39.es/ecma262/multipage/numbers-and-dates.html
- 3: https://402.ecma-international.org/1.0/
- 4: https://dev.ecma-international.org/publications-and-standards/standards/ecma-402/
- 5: https://402.ecma-international.org/3.0/
- 6: https://tc39.es/ecma402
- 7: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/NumberFormat/NumberFormat
- 8: https://tc39.es/proposal-intl-numberformat-v3/out/numberformat/proposed.html
- 9: https://rawgit.com/zbraniecki/ecma402/format-to-parts-mix/out/index.html
- 10: https://v8.dev/features/intl-numberformat
- 11: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat/NumberFormat
- 12: https://webplatform.github.io/docs/apis/internationalization/
- 13: https://devdoc.net/web/developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/NumberFormat/prototype.html
Verify minimum browser targets for Intl.NumberFormat string-precision support
The code passes strings to Intl.NumberFormat.format() for large-value precision preservation, relying on V3 spec behavior that treats strings with exact precision rather than converting to Number first. However, the library has no explicit browser version constraints (TypeScript target is "es6" with no browserslist), making it unclear whether all deployment environments meet the V3 adoption timeline (Chrome 106+, Firefox 116+, Safari 15.4+ — ~2022–2023).
Confirm that the minimum supported browsers include full Intl.NumberFormat V3 string-precision support, or add a feature-detection guard:
const supportsStringFormat = new Intl.NumberFormat('und').format('11111111111111111112').indexOf('2') !== -1;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/raystack/components/amount/amount.tsx` around lines 239 - 240, The
code calls Intl.NumberFormat.format(finalBaseValue) with finalBaseValue as a
string to preserve precision, but environments may not support V3
string-precision; either ensure your supported browsers include V3 (update
browserslist/README) or add a runtime feature-detection and fallback: implement
the suggested supportsStringFormat check (new
Intl.NumberFormat('und').format('11111111111111111112').indexOf('2') !== -1)
and, if false, convert finalBaseValue to a safe fallback (e.g.,
Number(finalBaseValue) or a BigInt-to-string grouping formatter) before calling
Intl.NumberFormat.format; update the code around the
Intl.NumberFormat.format(...) call in amount.tsx that uses finalBaseValue
accordingly.
1 participant
Description
Adds two capabilities to the Amount component and fixes one latent bug:
number | string.BigIntis treated as integer-only and always in major units, sovalueInMinorUnitsis ignored when value is aBigInt. This restores precision for amounts beyondNumber.MAX_SAFE_INTEGER (≈ 9 × 10¹⁵)— the previous workaround was string-only, which is awkward for backends that already produceBigInt. Intl.NumberFormat.prototype.format()acceptsBigIntnatively, so the integration is direct.hideCurrencyprop. A boolean that switches the formatter from style: 'currency' to style: 'decimal', rendering only the formatted number with locale-driven separators/grouping/fraction-digits intact. Useful for places where the currency is displayed elsewhere (column headers, line-item subtotals, token counters in the SDK Tokens view). WhenhideCurrencyis true,currencyDisplayis ignored.Behavior contract documented in JSDoc on both source and docs props.ts. Backwards compatible —
hideCurrencydefaults to false, theBigIntbranch is purely additive, and the negative-string path was previously buggy (no test ever passed against it).Type of Change
How Has This Been Tested?
BigInt (6),hideCurrency (4), and thenegative-string regression (1). Full suite at 41/41 passing.MAX_SAFE_INTEGER,negative BigInts,zero-decimal currency (JPY), noMAX_SAFE_INTEGERwarning forBigInt.hideCurrency: basic case, BigInt input, override of currencyDisplay, currency-driven decimals still respected.hideCurrencyDemoand aBigIntentry inlargeNumbersDemo. The console-warning demo value was bumped from 15 digits (which never tripped Math.abs > MAX_SAFE_INTEGER) to 17 digits, so the warning now actually fires when the section is rendered.Checklist:
Screenshots (if appropriate):
N/A
Related Issues
N/A