{{ message }}
fix: pin toLocaleString() calls to en-US locale - #1400
Merged
kishore08-07 merged 3 commits intoAug 4, 2026
Conversation
Several number formatting call sites relied on the host machine's default locale instead of a fixed one, causing inconsistent digit grouping in the UI and non-deterministic test failures on non-US locales.
Collaborator
kishore08-07
reviewed
Aug 3, 2026
kishore08-07
left a comment
kishore08-07
left a comment
Member
There was a problem hiding this comment.
Thanks for the PR! While this fixes the test failures, but hardcoding 'en-US' directly in the ui components has a few drawbacks:
- breaks i18n support,
- could lead to inconsistent ui: balances (btc/sats) will still format using the user's browser locale, creating a mix of formatting styles on the same screen.
Contributor
Author
|
Thanks both for the feedback. good points on hardcoding the locale in components. I've reworked this:
Tests now pass deterministically regardless of the machine's locale, and the UI behavior is unchanged. Let me know if this matches what you had in mind. |
theborakompanioni
approved these changes
Aug 4, 2026
theborakompanioni
left a comment
theborakompanioni
left a comment
Collaborator
There was a problem hiding this comment.
utACK.
CI would need to run in a different locale than en-US to be sure this does not break again. Good follow-up?
Contributor
Author
kishore08-07
approved these changes
Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1398
Pins the remaining unpinned
toLocaleString()calls (block heights, fee rates, offer/fidelity-bond sizes, scheduled-transaction counts) toen-US, so number formatting is consistent regardless of the host machine's locale.Also fixed
ImportDetailsForm.test.tsx, which built its expected string using an unpinnedtoLocaleString()and would otherwise mismatch the now-pinned component output.Tested:
npx vitest run --project unit— 264/264 relevant tests pass.tsc -bbuild clean.