Skip to content

Commit 8f88c5d

Browse files
mountinycursoragent
andcommitted
Dedupe NumberFormatUtils try/catch and sanitize remaining Intl callers
- Extract createFormatter helper in NumberFormatUtils/index.ts so format and formatToParts share a single try/catch + USD fallback path (addresses the CONSISTENCY-3 bot review on discussion_r3274381328). - Apply sanitizeCurrencyCode to the two remaining direct callers that bypassed layer-1 validation: the convertToDisplayString and convertToDisplayStringWithoutCurrency closures in CurrencyListContextProvider, and the formatToParts call in SearchChartView. These previously only had the anonymous layer-2 catch as a safety net and missed normalization (e.g. 'eur' rendering as USD). - Add resetInvalidCurrencyWarningsForTesting in CurrencyUtils.ts so Log.warn-asserting tests can clear the module-level throttle Set in beforeEach instead of relying on globally-unique fixture strings. - Add a cross-helper throttle test asserting the shared Set deduplicates warnings across convertToDisplayString, getLocalizedCurrencySymbol, and convertToShortDisplayString for the same malformed code. - Add malformed-currency test matrix for convertToDisplayStringWithExplicitCurrency, including the falsy branch that delegates to convertToDisplayStringWithoutCurrency. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent f9e62db commit 8f88c5d

5 files changed

Lines changed: 64 additions & 30 deletions

File tree

src/components/CurrencyListContextProvider/index.tsx

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React, {createContext, useCallback, useContext, useMemo} from 'react';
22
import useLocalize from '@hooks/useLocalize';
33
import useOnyx from '@hooks/useOnyx';
4-
import {convertToFrontendAmountAsInteger} from '@libs/CurrencyUtils';
4+
import {convertToFrontendAmountAsInteger, sanitizeCurrencyCode} from '@libs/CurrencyUtils';
55
import {format, formatToParts} from '@libs/NumberFormatUtils';
66
import CONST from '@src/CONST';
77
import ONYXKEYS from '@src/ONYXKEYS';
@@ -34,15 +34,12 @@ function CurrencyListContextProvider({children}: React.PropsWithChildren) {
3434

3535
const convertToDisplayString = useCallback(
3636
(amountInCents: number | undefined, currencyCode: string | undefined): string => {
37-
const decimals = getCurrencyDecimals(currencyCode);
37+
const sanitizedCurrency = sanitizeCurrencyCode(currencyCode);
38+
const decimals = getCurrencyDecimals(sanitizedCurrency);
3839
const convertedAmount = convertToFrontendAmountAsInteger(amountInCents ?? 0, decimals);
39-
let currencyWithFallback = currencyCode;
40-
if (!currencyCode) {
41-
currencyWithFallback = CONST.CURRENCY.USD;
42-
}
4340
return format(preferredLocale, convertedAmount, {
4441
style: 'currency',
45-
currency: currencyWithFallback,
42+
currency: sanitizedCurrency,
4643

4744
// We are forcing the number of decimals because we override the default number of decimals in the backend for some currencies
4845
// See: https://github.com/Expensify/PHP-Libs/pull/834
@@ -56,11 +53,12 @@ function CurrencyListContextProvider({children}: React.PropsWithChildren) {
5653

5754
const convertToDisplayStringWithoutCurrency = useCallback(
5855
(amountInCents: number, currencyCode: string = CONST.CURRENCY.USD): string => {
59-
const decimals = getCurrencyDecimals(currencyCode);
56+
const sanitizedCurrency = sanitizeCurrencyCode(currencyCode);
57+
const decimals = getCurrencyDecimals(sanitizedCurrency);
6058
const convertedAmount = convertToFrontendAmountAsInteger(amountInCents, decimals);
6159
return formatToParts(preferredLocale, convertedAmount, {
6260
style: 'currency',
63-
currency: currencyCode,
61+
currency: sanitizedCurrency,
6462

6563
// We are forcing the number of decimals because we override the default number of decimals in the backend for some currencies
6664
// See: https://github.com/Expensify/PHP-Libs/pull/834

src/components/Search/SearchChartView.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import React from 'react';
22
import useLocalize from '@hooks/useLocalize';
3-
import {getCurrencySymbol} from '@libs/CurrencyUtils';
3+
import {getCurrencySymbol, sanitizeCurrencyCode} from '@libs/CurrencyUtils';
44
import Log from '@libs/Log';
55
import Navigation from '@libs/Navigation/Navigation';
66
import {formatToParts} from '@libs/NumberFormatUtils';
@@ -71,7 +71,7 @@ function SearchChartView({queryJSON, view, groupBy, data, isLoading}: SearchChar
7171
};
7272

7373
const firstItem = data.at(0);
74-
const currency = firstItem?.currency ?? 'USD';
74+
const currency = sanitizeCurrencyCode(firstItem?.currency ?? 'USD');
7575
const parts = formatToParts(preferredLocale, 0, {style: 'currency', currency});
7676
const currencyIndex = parts.findIndex((p) => p.type === 'currency');
7777
const integerIndex = parts.findIndex((p) => p.type === 'integer');

src/libs/CurrencyUtils.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,14 @@ function isValidCurrencyCode(currencyCode: unknown): currencyCode is string {
3131
// Tracks invalid currency codes already warned about so the same bad value doesn't spam the log on every render.
3232
const warnedInvalidCurrencyCodes = new Set<string>();
3333

34+
/**
35+
* Test-only: clears the in-memory de-dup of malformed currency codes so tests asserting on `Log.warn`
36+
* are not affected by warnings emitted by earlier tests. Production code should not call this.
37+
*/
38+
function resetInvalidCurrencyWarningsForTesting() {
39+
warnedInvalidCurrencyCodes.clear();
40+
}
41+
3442
/**
3543
* Validates a currency code and returns it unchanged if it is a valid ISO 4217 code.
3644
* Whitespace and case-only variations (e.g. " usd ") are normalized rather than discarded.
@@ -230,6 +238,7 @@ function convertToDisplayStringWithoutCurrency(amountInCents: number, currency:
230238
export {
231239
isValidCurrencyCode,
232240
sanitizeCurrencyCode,
241+
resetInvalidCurrencyWarningsForTesting,
233242
getCurrencyDecimals,
234243
getCurrencyUnit,
235244
getLocalizedCurrencySymbol,

src/libs/NumberFormatUtils/index.ts

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,30 +9,29 @@ intlPolyfill();
99

1010
const MemoizedNumberFormat = memoize(Intl.NumberFormat, {maxSize: 10, monitoringName: 'NumberFormatUtils'});
1111

12-
function format(locale: Locale | undefined, number: number, options?: Intl.NumberFormatOptions): string {
12+
/**
13+
* Build an Intl.NumberFormat with a USD fallback for malformed currency codes.
14+
* Intl throws RangeError for empty/malformed currency values; check presence of the option (not truthiness)
15+
* so we still recover when currency is '' rather than rethrowing and crashing the screen.
16+
*/
17+
function createFormatter(locale: Locale | undefined, options?: Intl.NumberFormatOptions): Intl.NumberFormat {
1318
try {
14-
return new MemoizedNumberFormat(locale ?? CONST.LOCALES.DEFAULT, options).format(number);
19+
return new MemoizedNumberFormat(locale ?? CONST.LOCALES.DEFAULT, options);
1520
} catch (e) {
16-
// Intl throws RangeError for empty/malformed currency values; check presence of the option (not truthiness)
17-
// so we still recover when currency is '' rather than rethrowing and crashing the screen.
1821
if (e instanceof RangeError && options && 'currency' in options) {
1922
Log.warn('NumberFormatUtils: malformed currency code, falling back to USD', {currency: options.currency});
20-
return new MemoizedNumberFormat(locale ?? CONST.LOCALES.DEFAULT, {...options, currency: CONST.CURRENCY.USD}).format(number);
23+
return new MemoizedNumberFormat(locale ?? CONST.LOCALES.DEFAULT, {...options, currency: CONST.CURRENCY.USD});
2124
}
2225
throw e;
2326
}
2427
}
2528

29+
function format(locale: Locale | undefined, number: number, options?: Intl.NumberFormatOptions): string {
30+
return createFormatter(locale, options).format(number);
31+
}
32+
2633
function formatToParts(locale: Locale | undefined, number: number, options?: Intl.NumberFormatOptions): Intl.NumberFormatPart[] {
27-
try {
28-
return new MemoizedNumberFormat(locale ?? CONST.LOCALES.DEFAULT, options).formatToParts(number);
29-
} catch (e) {
30-
if (e instanceof RangeError && options && 'currency' in options) {
31-
Log.warn('NumberFormatUtils: malformed currency code, falling back to USD', {currency: options.currency});
32-
return new MemoizedNumberFormat(locale ?? CONST.LOCALES.DEFAULT, {...options, currency: CONST.CURRENCY.USD}).formatToParts(number);
33-
}
34-
throw e;
35-
}
34+
return createFormatter(locale, options).formatToParts(number);
3635
}
3736

3837
export {format, formatToParts};

tests/unit/CurrencyUtilsTest.ts

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,10 @@ describe('CurrencyUtils', () => {
214214
});
215215

216216
describe('sanitizeCurrencyCode', () => {
217+
beforeEach(() => {
218+
CurrencyUtils.resetInvalidCurrencyWarningsForTesting();
219+
});
220+
217221
test('returns the input unchanged for a valid ISO 4217 code', () => {
218222
expect(CurrencyUtils.sanitizeCurrencyCode('EUR')).toBe('EUR');
219223
});
@@ -240,13 +244,12 @@ describe('CurrencyUtils', () => {
240244
test('logs a warning at most once per unique malformed value', () => {
241245
const warnSpy = jest.spyOn(Log, 'warn').mockImplementation(() => undefined);
242246
try {
243-
// Use unique fixtures so other tests don't already mark these as warned.
244-
CurrencyUtils.sanitizeCurrencyCode('TEST_THROTTLE_A');
245-
CurrencyUtils.sanitizeCurrencyCode('TEST_THROTTLE_A');
246-
CurrencyUtils.sanitizeCurrencyCode('TEST_THROTTLE_A');
247+
CurrencyUtils.sanitizeCurrencyCode('XX');
248+
CurrencyUtils.sanitizeCurrencyCode('XX');
249+
CurrencyUtils.sanitizeCurrencyCode('XX');
247250
expect(warnSpy).toHaveBeenCalledTimes(1);
248251

249-
CurrencyUtils.sanitizeCurrencyCode('TEST_THROTTLE_B');
252+
CurrencyUtils.sanitizeCurrencyCode('???');
250253
expect(warnSpy).toHaveBeenCalledTimes(2);
251254
} finally {
252255
warnSpy.mockRestore();
@@ -262,6 +265,18 @@ describe('CurrencyUtils', () => {
262265
warnSpy.mockRestore();
263266
}
264267
});
268+
269+
test('shares the throttle across helpers that go through sanitizeCurrencyCode', () => {
270+
const warnSpy = jest.spyOn(Log, 'warn').mockImplementation(() => undefined);
271+
try {
272+
CurrencyUtils.convertToDisplayString(2500, 'XX');
273+
CurrencyUtils.getLocalizedCurrencySymbol(CONST.LOCALES.EN, 'XX');
274+
CurrencyUtils.convertToShortDisplayString(2500, 'XX');
275+
expect(warnSpy).toHaveBeenCalledTimes(1);
276+
} finally {
277+
warnSpy.mockRestore();
278+
}
279+
});
265280
});
266281

267282
describe('convertToDisplayString with malformed currency', () => {
@@ -310,6 +325,19 @@ describe('CurrencyUtils', () => {
310325
});
311326
});
312327

328+
describe('convertToDisplayStringWithExplicitCurrency with malformed currency', () => {
329+
test.each(['XX', 'USDD', '???'])('does not throw and falls back to USD formatting for truthy malformed %p', (input) => {
330+
expect(() => CurrencyUtils.convertToDisplayStringWithExplicitCurrency(2500, input)).not.toThrow();
331+
expect(CurrencyUtils.convertToDisplayStringWithExplicitCurrency(2500, input)).toBe('$25.00');
332+
});
333+
334+
test.each([undefined, ''])('returns the symbol-less form for falsy currency %p (delegates to convertToDisplayStringWithoutCurrency)', (input) => {
335+
const result = CurrencyUtils.convertToDisplayStringWithExplicitCurrency(2500, input);
336+
expect(result).not.toMatch(/\$/);
337+
expect(result).toContain('25');
338+
});
339+
});
340+
313341
describe('getLocalizedCurrencySymbol with malformed currency', () => {
314342
test.each(['', 'XX', 'USDD'])('returns the USD symbol without throwing for %p', (input) => {
315343
expect(() => CurrencyUtils.getLocalizedCurrencySymbol(CONST.LOCALES.EN, input)).not.toThrow();

0 commit comments

Comments
 (0)