Paywalls: new .onPurchaseStarted(package) modifier#3693
Conversation
Paywalls: new .onPurchaseStarted(package) modifier
216b956 to
c8e9e65
Compare
While working on #3693 I noticed we were missing some API tests that would be helpful for that PR
3cd5f28 to
d3b028d
Compare
Since we are deprecating the old function in https://github.com/RevenueCat/purchases-ios/pull/3693/files I thought it will be simpler to just add both `restoreStarted` and the new `purchaseStarted(package:)` in the same release. We haven't made a release since I merged #3694 so these are safe to remove for now. I open this PR in case we want to make a release before #3693 is finished.
41d9b2d to
d13cc83
Compare
ca20c3f to
2cccdc3
Compare
| fonts: PaywallFontProvider = DefaultPaywallFontProvider(), | ||
| presentationMode: PaywallPresentationMode = .default, | ||
| purchaseStarted: PurchaseStartedHandler? = nil, | ||
| purchaseStarted: @escaping PurchaseStartedHandler, |
There was a problem hiding this comment.
Made this non nullable so the function wouldn't collide if someone was only passing requiredEntitlementIdentifier. I also had to made it @escaping. Is that considered a breaking change?
There was a problem hiding this comment.
@vegaro I'm not certain if the @escaping is breaking changing (I'd have to experiment) but making this non-nullable is a breaking change, isn't it? 😬
If somebody was using this function without purchaseStarted, it will now require them to have purchaseStarted?
There was a problem hiding this comment.
Did the API tester for this break? That would really let us know, wouldn't it? 🤷♂️
There was a problem hiding this comment.
It didn't fail, that's why I am not sure 🤷♂️
If somebody was using this function without purchaseStarted, it will now require them to have purchaseStarted?
If they were not passing it, it will use the new non-deprecated function automatically (that one has it as nullable)
There was a problem hiding this comment.
Ahhh, okay! That makes sense then 😊 I think this should be good then!
| presentationMode: PaywallPresentationMode = .default, | ||
| shouldDisplay: @escaping @Sendable (CustomerInfo) -> Bool, | ||
| purchaseStarted: PurchaseStartedHandler? = nil, | ||
| purchaseStarted: @escaping PurchaseStartedHandler, |
There was a problem hiding this comment.
Same here. If someone was not passing anything or passing let's say just offering, it would collide with the new version
tonidero
left a comment
There was a problem hiding this comment.
LGTM! I don't think the escaping change is breaking but might be good to confirm with @joshdholtz and/or @MarkVillacampa
| return self.modifier(OnPurchaseStartedModifier(handler: { _ in | ||
| handler() | ||
| })) | ||
| } |
There was a problem hiding this comment.
Should we mark this one as deprecated as well?
| fonts: PaywallFontProvider = DefaultPaywallFontProvider(), | ||
| presentationMode: PaywallPresentationMode = .default, | ||
| purchaseStarted: PurchaseStartedHandler? = nil, | ||
| purchaseStarted: @escaping PurchaseStartedHandler, |
There was a problem hiding this comment.
@vegaro I'm not certain if the @escaping is breaking changing (I'd have to experiment) but making this non-nullable is a breaking change, isn't it? 😬
If somebody was using this function without purchaseStarted, it will now require them to have purchaseStarted?
| .onPurchaseStarted { | ||
| started = true | ||
| } | ||
| .onPurchaseStarted { package in |
There was a problem hiding this comment.
Does this work for a variable name?! Even though package is a reserved word? 😅
There was a problem hiding this comment.
I know, it doesn't complain so I think so?
| fonts: PaywallFontProvider = DefaultPaywallFontProvider(), | ||
| presentationMode: PaywallPresentationMode = .default, | ||
| purchaseStarted: PurchaseStartedHandler? = nil, | ||
| purchaseStarted: @escaping PurchaseStartedHandler, |
There was a problem hiding this comment.
Did the API tester for this break? That would really let us know, wouldn't it? 🤷♂️
Similar to #3627.
This API is available in:
PaywallView().presentPaywallIfNeeded.paywallFooterPaywallViewControllerDelegate