Skip to content

Commit dbcb3c8

Browse files
mountinycursoragent
andcommitted
Address PR feedback: throttle NumberFormatUtils warn, polish
- Replace literal 'USD' in SearchChartView with CONST.CURRENCY.USD per CONSISTENCY-2 (FitseTLT and github-actions bot flagged the same line). - Clarify the createFormatter JSDoc in NumberFormatUtils per FitseTLT's comment: split the general purpose from the empty-string nuance so the two ideas don't read as one run-on sentence. - Throttle the Log.warn in createFormatter via a local Set so the safety-net log doesn't fire on every render for the same malformed currency. Mirrors the pattern in CurrencyUtils.sanitizeCurrencyCode but uses a local Set to avoid a circular import (CurrencyUtils already imports NumberFormatUtils). - Export resetMalformedCurrenciesForTesting and call it in beforeEach in NumberFormatUtilsTest so warn-assertion tests stay isolated. - Add two tests: same malformed currency warns only once across repeated format calls, and the deduplication is shared between format and formatToParts. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 8f88c5d commit dbcb3c8

3 files changed

Lines changed: 41 additions & 6 deletions

File tree

src/components/Search/SearchChartView.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ function SearchChartView({queryJSON, view, groupBy, data, isLoading}: SearchChar
7171
};
7272

7373
const firstItem = data.at(0);
74-
const currency = sanitizeCurrencyCode(firstItem?.currency ?? 'USD');
74+
const currency = sanitizeCurrencyCode(firstItem?.currency ?? CONST.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/NumberFormatUtils/index.ts

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,26 @@ intlPolyfill();
99

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

12+
// Tracks malformed currency codes that have already produced a warning in this module, so the
13+
// safety-net log doesn't fire on every render for the same bad value.
14+
const warnedMalformedCurrencies = new Set<string>();
15+
1216
/**
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.
17+
* Build an Intl.NumberFormat. If construction throws a RangeError due to a malformed currency
18+
* code (Intl rejects empty strings and non-ISO-4217 codes), fall back to USD instead of letting
19+
* the error crash the screen. We check `'currency' in options` rather than `options.currency`
20+
* truthiness so an empty-string currency still triggers the fallback.
1621
*/
1722
function createFormatter(locale: Locale | undefined, options?: Intl.NumberFormatOptions): Intl.NumberFormat {
1823
try {
1924
return new MemoizedNumberFormat(locale ?? CONST.LOCALES.DEFAULT, options);
2025
} catch (e) {
2126
if (e instanceof RangeError && options && 'currency' in options) {
22-
Log.warn('NumberFormatUtils: malformed currency code, falling back to USD', {currency: options.currency});
27+
const currency = String(options.currency ?? '');
28+
if (!warnedMalformedCurrencies.has(currency)) {
29+
warnedMalformedCurrencies.add(currency);
30+
Log.warn('NumberFormatUtils: malformed currency code, falling back to USD', {currency: options.currency});
31+
}
2332
return new MemoizedNumberFormat(locale ?? CONST.LOCALES.DEFAULT, {...options, currency: CONST.CURRENCY.USD});
2433
}
2534
throw e;
@@ -34,4 +43,11 @@ function formatToParts(locale: Locale | undefined, number: number, options?: Int
3443
return createFormatter(locale, options).formatToParts(number);
3544
}
3645

37-
export {format, formatToParts};
46+
/**
47+
* Test-only: clears the malformed-currency deduplication so warn-assertion tests don't pollute each other.
48+
*/
49+
function resetMalformedCurrenciesForTesting() {
50+
warnedMalformedCurrencies.clear();
51+
}
52+
53+
export {format, formatToParts, resetMalformedCurrenciesForTesting};

tests/unit/NumberFormatUtilsTest.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ describe('NumberFormatUtils', () => {
66
let warnSpy: jest.SpyInstance;
77

88
beforeEach(() => {
9+
NumberFormatUtils.resetMalformedCurrenciesForTesting();
910
warnSpy = jest.spyOn(Log, 'warn').mockImplementation(() => undefined);
1011
});
1112

@@ -82,4 +83,22 @@ describe('NumberFormatUtils', () => {
8283
expect(() => NumberFormatUtils.formatToParts(CONST.LOCALES.EN, 0, {style: 'unit', unit: 'not-a-real-unit'} as Intl.NumberFormatOptions)).toThrow(RangeError);
8384
});
8485
});
86+
87+
describe('malformed-currency warn deduplication', () => {
88+
test('warns at most once per unique malformed currency across repeated format calls', () => {
89+
NumberFormatUtils.format(CONST.LOCALES.EN, 25, {style: 'currency', currency: 'XX'});
90+
NumberFormatUtils.format(CONST.LOCALES.EN, 25, {style: 'currency', currency: 'XX'});
91+
NumberFormatUtils.format(CONST.LOCALES.EN, 25, {style: 'currency', currency: 'XX'});
92+
expect(warnSpy).toHaveBeenCalledTimes(1);
93+
94+
NumberFormatUtils.format(CONST.LOCALES.EN, 25, {style: 'currency', currency: '???'});
95+
expect(warnSpy).toHaveBeenCalledTimes(2);
96+
});
97+
98+
test('the deduplication is shared between format and formatToParts', () => {
99+
NumberFormatUtils.format(CONST.LOCALES.EN, 25, {style: 'currency', currency: 'YY'});
100+
NumberFormatUtils.formatToParts(CONST.LOCALES.EN, 0, {style: 'currency', currency: 'YY'});
101+
expect(warnSpy).toHaveBeenCalledTimes(1);
102+
});
103+
});
85104
});

0 commit comments

Comments
 (0)