bito-code-review[bot] commented on PR #40931:
URL: https://github.com/apache/superset/pull/40931#issuecomment-4668779806
<!-- Bito Reply -->
The performance issue flagged in `resolveSymbolPosition` is valid.
Constructing a new `Intl.NumberFormat` instance on every call within a hot
formatting path is indeed inefficient. To resolve this, you can implement a
simple cache using a `Map` keyed by a combination of `locale` and
`currencyCode`.
Here is a concise implementation to cache the resolved `SymbolPosition`:
```typescript
const positionCache = new Map<string, SymbolPosition>();
export function resolveSymbolPosition(
currencyCode: string | undefined,
symbolPosition?: string,
locale: string = getCurrencyLocale(),
): SymbolPosition {
if (symbolPosition === 'prefix' || symbolPosition === 'suffix') {
return symbolPosition;
}
const cacheKey = `${locale}:${currencyCode}`;
if (positionCache.has(cacheKey)) {
return positionCache.get(cacheKey)!;
}
let position: SymbolPosition = 'prefix';
if (currencyCode) {
try {
const parts = new Intl.NumberFormat(locale, {
style: 'currency',
currency: currencyCode,
}).formatToParts(1);
const currencyIndex = parts.findIndex(part => part.type ===
'currency');
const valueIndex = parts.findIndex(part =>
NUMERIC_PART_TYPES.has(part.type));
if (currencyIndex !== -1 && valueIndex !== -1) {
position = currencyIndex < valueIndex ? 'prefix' : 'suffix';
}
} catch {}
}
positionCache.set(cacheKey, position);
return position;
}
```
Regarding other comments on this PR: there are no other review comments in
the provided context. If you have additional comments you would like me to
review, please provide them.
**superset-frontend/packages/superset-ui-core/src/currency-format/symbolPosition.ts**
```
const positionCache = new Map<string, SymbolPosition>();
export function resolveSymbolPosition(
currencyCode: string | undefined,
symbolPosition?: string,
locale: string = getCurrencyLocale(),
): SymbolPosition {
if (symbolPosition === 'prefix' || symbolPosition === 'suffix') {
return symbolPosition;
}
const cacheKey = `${locale}:${currencyCode}`;
if (positionCache.has(cacheKey)) {
return positionCache.get(cacheKey)!;
}
let position: SymbolPosition = 'prefix';
if (currencyCode) {
try {
const parts = new Intl.NumberFormat(locale, {
style: 'currency',
currency: currencyCode,
}).formatToParts(1);
const currencyIndex = parts.findIndex(part => part.type ===
'currency');
const valueIndex = parts.findIndex(part =>
NUMERIC_PART_TYPES.has(part.type));
if (currencyIndex !== -1 && valueIndex !== -1) {
position = currencyIndex < valueIndex ? 'prefix' : 'suffix';
}
} catch {}
}
positionCache.set(cacheKey, position);
return position;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
