fix(explorer): formatPercentage - force "." decimal symbol for any locale

[crutch12]

Summary

yarn test fails some tests if developer's OS has non en-US Intl.NumberFormat locale (e.g. with ru-RU or de-DE)

This is because of formatSmart usage:

import { formatSmart, safeTokenName, TokenErc20 } from '@gnosis.pm/dex-js'

How it generally works:

const number = 123456.789;
const userNumberFormat = new Intl.NumberFormat().format(number);
console.log(userNumberFormat); // e.g., "123,456.789" (for en-US) or "123.456,789" (for de-DE)

So formatSmart preserves user's locale settings and it works correctly. But we don't need user's locale decimal for percentages, we need "." decimal .

So every formatSmart() should be called with isLocaleAware: false, when it's called from formatPercentage

What I did:

1. inside formatPercentage return formatSmart() with isLocaleAware: false
2. added different tests with locales to failed test files

Now

1. all tests pass
2. formatPercentage always returns value with "." decimal

To Test

1. yarn test shouldn't fail with any Intl.NumberFormat locale

Background

• It would be great to use

import { formatPercent } from '@cowprotocol/common-utils'

instead of own formatPercentage implementation

• tests: mockNumberLocale should be called in advance, so formattedAmount/formatPercentage are imported dynamically
• tests: mockNumberLocale is located in separated file, because it shouldn't import another utils files

Summary by CodeRabbit

• Bug Fixes

  • Percentage values now show consistent decimal formatting (dot as decimal separator) across all locales.

• Tests

  • Added locale-aware testing and a mock helper to validate number formatting across multiple regional settings.
  • Updated tests to run formatting checks for multiple locales and restore locale mocks between runs.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[vercel]

@crutch12 is attempting to deploy a commit to the cow Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

Walkthrough

Adds a test helper to mock Intl.NumberFormat, refactors two unit tests to run across multiple locales using dynamic imports and lifecycle hooks, and forces locale-agnostic decimal symbol in formatPercentage by passing isLocaleAware: false to formatSmart.

Changes

Cohort / File(s)	Summary
Test helper apps/explorer/src/test/mockNumberLocale.ts	New module exporting mockNumberLocale(locale: string): jest.SpyInstance that spies on Intl.NumberFormat to return a locale-specific formatter for tests.
Locale-aware tests apps/explorer/src/test/utils/format/formatPercentage.spec.ts	Refactored tests to iterate over multiple locales, added beforeEach/afterEach to reset modules and restore mocks, and switched to dynamic import of formatPercentage after applying the locale mock; test titles now include locale.
Locale-aware tests apps/explorer/src/test/utils/operator/orderFormatAmount.test.ts	Replaced runtime import of TokenErc20 with a type-only import, added locale mock lifecycle (reset modules, apply en-US by default, restore), converted tests to async, and moved imports of formatting utilities to dynamic imports executed after locale mocks.
Formatting logic apps/explorer/src/utils/format.ts	In formatPercentage, added isLocaleAware: false to the formatSmart call to force the decimal symbol to . regardless of active locale.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• Verify lifecycle hooks (beforeEach/afterEach) correctly reset modules and restore spies in both test files.
• Confirm dynamic imports occur after locale mocking so formatting uses the mocked Intl.NumberFormat.
• Check that isLocaleAware: false in formatPercentage produces expected outputs across mocked locales.

Poem

🐰 I sniff the decimals, hop by hop,
Mocking locales until they stop,
Tests dance through commas, dots, and space,
One rabbit cheer for a steadier place,
Formatting neat — a joyful hop! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: forcing formatPercentage to use '.' decimal symbol regardless of locale.
Description check	✅ Passed	PR description follows the template with clear Summary, To Test, and Background sections explaining the locale issue and solution.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

apps/explorer/src/test/mockNumberLocale.ts (1)

1-4: Locale mock helper is simple and effective; consider forwarding args if needs grow

This helper cleanly centralizes locale mocking and ensures tests see deterministic Intl.NumberFormat behavior per locale. One minor future‑proofing idea: if any code under test starts relying on Intl.NumberFormat options, you may want the mock implementation to accept and forward (...args) rather than ignoring parameters, so behavior stays closer to the real API; for the current use (just decimal symbol), this is fine.

If you later expand tests that depend on Intl.NumberFormat options (e.g., maximumFractionDigits), double‑check that this mock still matches expectations.

apps/explorer/src/test/utils/operator/orderFormatAmount.test.ts (1)

2-49: Order amount tests align well with new locale‑aware pattern

Using mockNumberLocale('en-US') in beforeEach and dynamically importing formattedAmount after the mock is applied is consistent with the new locale‑aware testing approach and should make these tests stable across developer locales. The type‑only TokenErc20 import and isTokenErc20 checks look correct for the simple ERC20 shape you’re exercising. If you want to tighten things further, you could drop the unnecessary async keyword from tests that don’t await anything, but that’s purely cosmetic.

Please re-run this suite with a non‑en‑US OS locale to confirm formattedAmount assertions remain stable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ce4d7e3 and 080d39c.

📒 Files selected for processing (4)

• apps/explorer/src/test/mockNumberLocale.ts (1 hunks)
• apps/explorer/src/test/utils/format/formatPercentage.spec.ts (2 hunks)
• apps/explorer/src/test/utils/operator/orderFormatAmount.test.ts (2 hunks)
• apps/explorer/src/utils/format.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (5)

📓 Common learnings

Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx:23-33
Timestamp: 2025-07-24T16:42:53.154Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx, the use of toFixed(2) for percentage formatting in tooltip content is intentional and differs from the banner message formatting that uses toSignificant(2, undefined, Rounding.ROUND_DOWN). This formatting difference serves different UX purposes and should not be flagged as inconsistent.

Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/highFeeWarningHelpers.ts:18-20
Timestamp: 2025-07-24T16:43:47.639Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/highFeeWarningHelpers.ts, the formatFeePercentage function uses ROUND_DOWN with toSignificant(2) for "at least X%" messaging. This ensures the displayed percentage is never higher than the actual fee, making the "at least" phrasing accurate. For example, if the actual fee is 25.4%, displaying "at least 25%" is correct, but "at least 26%" would be misleading.

📚 Learning: 2025-07-24T16:42:53.154Z

Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx:23-33
Timestamp: 2025-07-24T16:42:53.154Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx, the use of toFixed(2) for percentage formatting in tooltip content is intentional and differs from the banner message formatting that uses toSignificant(2, undefined, Rounding.ROUND_DOWN). This formatting difference serves different UX purposes and should not be flagged as inconsistent.

Applied to files:

• apps/explorer/src/utils/format.ts

📚 Learning: 2025-07-24T16:43:47.639Z

Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/highFeeWarningHelpers.ts:18-20
Timestamp: 2025-07-24T16:43:47.639Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/highFeeWarningHelpers.ts, the formatFeePercentage function uses ROUND_DOWN with toSignificant(2) for "at least X%" messaging. This ensures the displayed percentage is never higher than the actual fee, making the "at least" phrasing accurate. For example, if the actual fee is 25.4%, displaying "at least 25%" is correct, but "at least 26%" would be misleading.

Applied to files:

• apps/explorer/src/utils/format.ts

📚 Learning: 2025-03-10T16:03:11.687Z

Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 5495
File: apps/cowswap-frontend/src/legacy/state/orders/utils.ts:444-452
Timestamp: 2025-03-10T16:03:11.687Z
Learning: When working with large numbers in JavaScript, especially token amounts with many decimals, avoid conversions through Number() before BigInt to prevent precision loss. Instead, pass string representations directly to JSBI.BigInt().

Applied to files:

• apps/explorer/src/utils/format.ts
• apps/explorer/src/test/utils/format/formatPercentage.spec.ts

📚 Learning: 2025-07-28T16:26:08.051Z

Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6034
File: apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts:44-47
Timestamp: 2025-07-28T16:26:08.051Z
Learning: In the cowswap codebase, using trivial placeholder tests like `it('should be true', () => { expect(true).to.be.true })` in e2e test files is an intentional pattern when disabling broken tests to keep CI green while maintaining build efficiency.

Applied to files:

• apps/explorer/src/test/utils/operator/orderFormatAmount.test.ts

🧬 Code graph analysis (1)

apps/explorer/src/test/utils/format/formatPercentage.spec.ts (2)

apps/explorer/src/test/mockNumberLocale.ts (1)

• mockNumberLocale (1-4)

apps/explorer/src/utils/format.ts (1)

• formatPercentage (114-134)

🔇 Additional comments (2)

apps/explorer/src/utils/format.ts (1)

125-133: formatPercentage: locale‑agnostic decimal looks correct

Using isLocaleAware: false here keeps the existing rounding/threshold behavior while guaranteeing "." as the decimal separator for all locales, which matches the PR objective and the new tests. No other callers are affected since this is scoped to percentage display only.

Please re-run the explorer test suite on a machine with a non‑en‑US locale to confirm there are no remaining locale‑dependent regressions.

apps/explorer/src/test/utils/format/formatPercentage.spec.ts (1)

3-42: Locale‑matrix tests and dynamic import pattern look solid

The test structure is good: you reset modules, mock Intl.NumberFormat per locale, then dynamically import formatPercentage, which ensures each test runs under its intended locale context. Iterating over both CASES and LOCALES gives strong coverage that the new isLocaleAware: false behavior consistently returns "." as the decimal separator regardless of environment locale.

Keep an eye on runtime if more cases/locales are added; if the matrix grows large, consider describe.each/it.each to keep Jest output readable and focused.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

apps/explorer/src/test/utils/format/formatPercentage.spec.ts (1)

36-42: Dynamic import per-locale is a good way to bind the mocked Intl

Deferring import('../../../utils') until after mockNumberLocale(locale) inside each it ensures formatPercentage is initialized under the correct mocked Intl.NumberFormat. The nested CASES.forEach + LOCALES.forEach structure is clear and achieves the goal of asserting locale-invariant "." decimals.

If you want, this could be slightly more declarative with describe.each(LOCALES) / it.each(CASES) in the future, but that’s purely stylistic.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 080d39c and 980db56.

📒 Files selected for processing (4)

• apps/explorer/src/test/mockNumberLocale.ts (1 hunks)
• apps/explorer/src/test/utils/format/formatPercentage.spec.ts (2 hunks)
• apps/explorer/src/test/utils/operator/orderFormatAmount.test.ts (2 hunks)
• apps/explorer/src/utils/format.ts (1 hunks)

✅ Files skipped from review due to trivial changes (1)

• apps/explorer/src/test/mockNumberLocale.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• apps/explorer/src/utils/format.ts
• apps/explorer/src/test/utils/operator/orderFormatAmount.test.ts

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx:23-33
Timestamp: 2025-07-24T16:42:53.154Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx, the use of toFixed(2) for percentage formatting in tooltip content is intentional and differs from the banner message formatting that uses toSignificant(2, undefined, Rounding.ROUND_DOWN). This formatting difference serves different UX purposes and should not be flagged as inconsistent.

Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/highFeeWarningHelpers.ts:18-20
Timestamp: 2025-07-24T16:43:47.639Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/highFeeWarningHelpers.ts, the formatFeePercentage function uses ROUND_DOWN with toSignificant(2) for "at least X%" messaging. This ensures the displayed percentage is never higher than the actual fee, making the "at least" phrasing accurate. For example, if the actual fee is 25.4%, displaying "at least 25%" is correct, but "at least 26%" would be misleading.

📚 Learning: 2025-03-10T16:03:11.687Z

Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 5495
File: apps/cowswap-frontend/src/legacy/state/orders/utils.ts:444-452
Timestamp: 2025-03-10T16:03:11.687Z
Learning: When working with large numbers in JavaScript, especially token amounts with many decimals, avoid conversions through Number() before BigInt to prevent precision loss. Instead, pass string representations directly to JSBI.BigInt().

Applied to files:

• apps/explorer/src/test/utils/format/formatPercentage.spec.ts

🧬 Code graph analysis (1)

apps/explorer/src/test/utils/format/formatPercentage.spec.ts (2)

apps/explorer/src/test/mockNumberLocale.ts (1)

• mockNumberLocale (1-4)

apps/explorer/src/utils/format.ts (1)

• formatPercentage (114-134)

🔇 Additional comments (2)

apps/explorer/src/test/utils/format/formatPercentage.spec.ts (2)

3-3: Import and LOCALES setup look correct

mockNumberLocale is imported from the expected test helper location and LOCALES covers a good spread of decimal formats (including RTL ar-SA), which should robustly catch regressions in locale handling. No changes needed.

Also applies to: 24-25

---

27-33: Lifecycle hooks correctly isolate locale/mocks between tests

Using jest.resetModules() in beforeEach together with jest.restoreAllMocks() in afterEach ensures that each test re-imports the module with a clean module registry and no leftover spies, which is important when mocking Intl.NumberFormat at import time. This is a solid pattern for this scenario; just be aware that resetModules can make tests a bit slower if many modules are loaded.

If you see any unexpected performance hit in this suite, consider checking Jest docs for jest.resetModules() vs alternatives like jest.isolateModules() to confirm this is still the recommended approach for your Jest version.