fix: use currencyDisplay: 'symbol' #22
Conversation
size-limit report 📦
|
📊 Package size report 0.6%↑
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
bashunaimiroy
changed the title
|
@joe-boudreau yes, it will only show it it doesn't match, like Canadian Dollars in Some interesting edge cases:
Anyways, see the tests for more details |
|
@kevinlee11 I've also updated it to cache the NumberFormat objects; otherwise we create a new one every time, which is a performance hit. This was called out by @rgershon , thanks Gershon |
| * The key is a string representing the locale and currency, | ||
| * and the value is the corresponding `Intl.NumberFormat` instance. | ||
| */ | ||
| #cachedFormatters: { [key: string]: Intl.NumberFormat } = {}; |
There was a problem hiding this comment.
this was suggested by @rgershon
There was a problem hiding this comment.
why? just curious what's going on behind the scenes here
There was a problem hiding this comment.
Is this something we're noticing initializing Intl.NumberFormat as being a heavy operation?
There was a problem hiding this comment.
I don't think so; But we're doing it a lot, and we're just trying to make Lightning Themes as performant as possible and this seemed like low hanging fruit
| ]; | ||
|
|
||
| currencyAndLocaleList.forEach(({ money, locale }) => { | ||
| expect(sdk.helpers.money.formatMoney(money, locale)).toBe(money.formatted); |
There was a problem hiding this comment.
The output of Intl.NumberFormat.format() is actually not guaranteed by the Javascript spec, and specifically MDN says we should not compare to a hardcoded string.
However, I think this is fine in a test case; if it starts failing, that would be good to know
This ensures that we see e.g. "CA $100", matching the twig money filter output
bashunaimiroy
force-pushed
the
currency-display
branch
from
69c417a
to
368380a
Compare
|
@kchung rebased and also marked this as a breaking change, since it changes the text output of a helper |
This ensures that we match the twig money filter output. For clarity about how this functions:
When the currency and locale match, we don't render the country abbreviation. E.g. 100 Canadian Dollars in
en-USlocale will be shown asCA$100, but inen-CAit'll be rendered just$100.