Skip to content
This repository has been archived by the owner on Oct 1, 2024. It is now read-only.

[react-i18n] Fix unformatCurrency utility stripping significant digits on RSD currency #1902

Merged
merged 5 commits into from
May 19, 2021

Conversation

lhoffbeck
Copy link
Contributor

@lhoffbeck lhoffbeck commented May 18, 2021

Description

Part of a fix to resolves Shopify/shopify issue #284738, along with shopify/web PR #43731.

RSD supports 2 decimal places, but (per Christian Jaekl)

...they've become worthless and fallen out of use. (1 para ~= $0.0001 USD currently, so 1RSD is approximately 1 US cent). ... Stripe still considers RSD to have subunits. In Chrome, Intl.NumberFormat treats RSD as if it has no decimals, which is what you're getting if you call formatCurrency without a form: parameter.

For example:

new Intl.NumberFormat('en-CA', {style: 'currency', currency: 'RSD'}).format('1.55')
	-> "RSD 2"
new Intl.NumberFormat('en-CA', {style: 'currency', currency: 'CAD', form: 'short}).format('1')
	-> "$1.55"

Because of this, when our unformatCurrency utility calls currencyDecimalSymbol, the formatCurrency rounds all RSD values to the nearest Int--unless you pass a form: short.

This change adds the form: short option to the currencyDecimalSymbol utility and updates the tests to check for this use case.

To 🎩 you can remove the change and the RSD test should fail.

Type of change

  • react-i18n Patch: Bug (non-breaking change which fixes an issue)
  • Minor: New feature (non-breaking change which adds functionality)
  • Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above (Documentation fix and Test update does not need a changelog as we do not publish new version)

@lhoffbeck lhoffbeck requested a review from a team May 18, 2021 18:55
Comment on lines 8 to 9
## Unreleased
- Fixed `currencyDecimalSymbol` function returning `DECIMAL_NOT_SUPPORTED` for RSD
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## Unreleased
- Fixed `currencyDecimalSymbol` function returning `DECIMAL_NOT_SUPPORTED` for RSD
<!-- Unreleased -->
- Fixed `currencyDecimalSymbol` function returning `DECIMAL_NOT_SUPPORTED` for RSD [#1902](https://github.com/Shopify/quilt/pull/1902)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw this just after I pushed the change up 🙈

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's more to update though :) The heading is a comment, and we recursively mention the PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh will fix :)

@atesgoral atesgoral requested a review from a team May 18, 2021 21:24
@lhoffbeck lhoffbeck merged commit 33dedbf into main May 19, 2021
@lhoffbeck lhoffbeck deleted the lh-react-i18n-fix-unformat-currency-for-rsd branch May 19, 2021 12:30
@atesgoral
Copy link
Contributor

@lhoffbeck @shopify/react-i18n@5.3.8 is released

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants