feat(amount): added bigInt and hideCurrency support in amount component#793
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR extends the Amount component with two complementary features. First, it broadens the value prop to accept bigint in addition to number and string, treating bigint values as already in major units (ignoring valueInMinorUnits). Second, it introduces a hideCurrency prop that renders only the formatted numeric portion while preserving locale-specific grouping and decimal separators. The implementation refactors the value-handling logic into explicit type branches and reworks the NumberFormat options to dynamically choose between decimal and currency styles. Tests validate edge cases including very large bigint values, negative inputs, and interactions with zero-decimal currencies. Sequence Diagram(s)sequenceDiagram
participant Client
participant AmountComponent
participant Intl as Intl.NumberFormat
participant Console
Client->>AmountComponent: render(value, {currency,...})
AmountComponent->>Intl: create formatter & format/formatToParts
Intl-->>AmountComponent: parts/string
AmountComponent->>Client: return formatted numeric string (strip currency parts if hideCurrency)
alt unsupported && large number precision concern
AmountComponent->>Console: warn about precision loss
end
Possibly related issues
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/amount.tsx`:
- Around line 252-257: The current computation of resolvedMinFrac and
resolvedMaxFrac in amount.tsx can produce an inverted range (min>max) when
hideCurrency is true and only one of minimumFractionDigits/maximumFractionDigits
is provided; update the logic in the resolvedMinFrac/resolvedMaxFrac assignments
(and the similar block at lines 259–270) to clamp the missing bound against the
provided one: if minimumFractionDigits is undefined but maximumFractionDigits is
defined, set resolvedMinFrac = Math.min(defaultDecimals, resolvedMaxFrac); if
maximumFractionDigits is undefined but minimumFractionDigits is defined, set
resolvedMaxFrac = Math.max(defaultDecimals, resolvedMinFrac); ensure both
resolvedMinFrac and resolvedMaxFrac are numbers and that resolvedMinFrac <=
resolvedMaxFrac before passing to Intl.NumberFormat to avoid RangeError.
🪄 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: 1777c7bb-4738-4365-a282-93a7e065ef31
📒 Files selected for processing (2)
packages/raystack/components/amount/__tests__/amount.test.tsxpackages/raystack/components/amount/amount.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/raystack/components/amount/tests/amount.test.tsx
|
@Shreyag02 Shouldn't this PR title be a |
Shreyag02
changed the title
3 participants
Description
Adds two capabilities to the Amount component:
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 renders only the formatted number, with locale-driven separators, grouping, and currency-driven fraction digits intact. Useful for places where the currency is displayed elsewhere (column headers, line-item subtotals, token counters in the SDK tokens view). When hideCurrency is true, currencyDisplay is 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