Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SK2StoreProduct: simplify currencyCode extraction #2485

Merged
merged 7 commits into from
May 23, 2023
Merged

Conversation

NachoSoto
Copy link
Contributor

@NachoSoto NachoSoto commented May 15, 2023

I noticed this new property thanks to this tweet.

This changes the "future" implementation of SK2StoreProduct.currencyCode to use the new (backdeployed) property.
We still need the existing implementation because @_backDeploy won't work if compiling with Xcode 13.x.

@NachoSoto NachoSoto added pr:feat A new feature refactor labels May 15, 2023
@NachoSoto NachoSoto requested a review from a team May 15, 2023 15:16
@NachoSoto
Copy link
Contributor Author

We should also look into exposing the Decimal.FormatStyle.Currency


var _currencyCode: String {
if #available(iOS 16.0, tvOS 16.0, watchOS 9.0, macOS 13.0, *) {
// This is marked as `@_backDeploy`, but it's not actually working before iOS 16.0
Copy link
Member

Choose a reason for hiding this comment

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

lol why am I not surprised

Copy link
Member

Choose a reason for hiding this comment

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

do we have a radar for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I figured out the reason: this only works in whatever version of the SDK added this, but you can't use if it you're compiling with Xcode 13.x obviously... but the docs don't even say when it was added.

@objc public var priceFormatter: NumberFormatter? { self.product.priceFormatter }
@objc public var priceFormatter: NumberFormatter { self.product.priceFormatter }
Copy link
Member

Choose a reason for hiding this comment

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

does this make it a breaking change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah which is why I made it a feat. But I had to revert this to fix Xcode 13.x compilation 🤷🏻‍♂️
Won't matter in the long term since you can't submit apps with that anymore, but we need to keep support for it in CI until I finish #2421.

@NachoSoto NachoSoto removed the pr:feat A new feature label May 15, 2023
@NachoSoto NachoSoto force-pushed the sk2-currency-code branch 2 times, most recently from c1d8ef5 to 875a31a Compare May 16, 2023 00:48
@NachoSoto NachoSoto requested review from a team and aboedo May 16, 2023 18:38
I noticed this new property thanks to [this tweet](https://twitter.com/ariskox/status/1658067009609904129?s=61&t=Zz639sMJdwHISm4AKYXoCw).

## Changes
- Removed `SK2StoreProduct.jsonDict`
- Obtaining `SK2StoreProduct.currencyCode` from [the new (backdeployed) property](https://developer.apple.com/documentation/storekit/product/4044347-priceformatstyle)
- Thanks to that, `StoreProductType.priceFormatter` no longer needs to be `Optional`

Because of that change, I'm making this a "feature" since it'll need a minor release, for users that are currently relying on that property being `Optional`.
@codecov
Copy link

codecov bot commented May 17, 2023

Codecov Report

Merging #2485 (2782837) into main (ce53ff4) will decrease coverage by 0.11%.
The diff coverage is 56.00%.

@@            Coverage Diff             @@
##             main    #2485      +/-   ##
==========================================
- Coverage   87.70%   87.59%   -0.11%     
==========================================
  Files         197      197              
  Lines       13327    13338      +11     
==========================================
- Hits        11689    11684       -5     
- Misses       1638     1654      +16     
Impacted Files Coverage Δ
...chasing/StoreKitAbstractions/SK2StoreProduct.swift 81.94% <52.17%> (-14.78%) ⬇️
...chasing/StoreKitAbstractions/SK1StoreProduct.swift 92.30% <100.00%> (-5.77%) ⬇️

... and 2 files with indirect coverage changes

@NachoSoto
Copy link
Contributor Author

This is ready now.

@NachoSoto NachoSoto merged commit c390a66 into main May 23, 2023
@NachoSoto NachoSoto deleted the sk2-currency-code branch May 23, 2023 21:33
NachoSoto added a commit that referenced this pull request May 25, 2023
I noticed this new property thanks to [this
tweet](https://twitter.com/ariskox/status/1658067009609904129?s=61&t=Zz639sMJdwHISm4AKYXoCw).

This changes the "future" implementation of
`SK2StoreProduct.currencyCode` to use [the new (backdeployed)
property](https://developer.apple.com/documentation/storekit/product/4044347-priceformatstyle).
We still need the existing implementation because `@_backDeploy` won't
work if compiling with `Xcode 13.x`.
This was referenced May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants