Skip to content

Observe PurchaseHandler when owned externally#4097

Merged
jamesrb1 merged 1 commit into
mainfrom
james/alternate-fix-for-clobbered-purchase-handler
Aug 8, 2024
Merged

Observe PurchaseHandler when owned externally#4097
jamesrb1 merged 1 commit into
mainfrom
james/alternate-fix-for-clobbered-purchase-handler

Conversation

@jamesrb1

Copy link
Copy Markdown
Contributor

Josh kindly fixed a problem while I was away where the purchase handler would get clobbered when the paywall was re-rendered. This PR restores avoiding having one struct owned by two @StateObjects at the same time.

@jamesrb1 jamesrb1 added the bug label Jul 22, 2024
@jamesrb1 jamesrb1 requested a review from joshdholtz July 22, 2024 21:07
@jamesrb1 jamesrb1 marked this pull request as ready for review July 22, 2024 21:08
@jamesrb1 jamesrb1 requested a review from a team July 22, 2024 21:24
@jamesrb1 jamesrb1 added pr:fix A bug fix and removed bug labels Jul 22, 2024

@joshdholtz joshdholtz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@jamesrb1 I think this looks good!

Does this still work with the overlay use case from the #4034?

Also, was there an example app use case where this fix was needed? Like where we observed some weird behavior? 🤔 Just want to make sure for documentation purposes incase we come across this again the in the future 😇

@jamesrb1

Copy link
Copy Markdown
Contributor Author

Does this still work with the #4034 (comment) from the #4034?

Yes!

Screen.Recording.2024-07-26.at.11.33.47.AM.mov

Also, was there an example app use case where this fix was needed? Like where we observed some weird behavior? 🤔 Just want to make sure for documentation purposes incase we come across this again the in the future 😇

No, I've observed no ill-behavior from having one object referenced by two @StateObject properties. Everything I've read says one thing should own it, everything else should observe it. If there are zero @StateObject references, you risk unexpected behaviour and crashes. But I have not seen anything that addresses the case of having multiple @StateObjects, but since that goes against the expected usage, I think the only safe way to consider this is that it's undefined behavior. (If the "second" view that owns it as a @StateObject is destroyed, does that cause the object to be destroyed too, when something else is still owning it? Or does this use a reference counting setup where it would still be OK? I don't know, and I don't know if it's defined anywhere so what it does could change in the future.)

@jamesrb1 jamesrb1 merged commit 3c08475 into main Aug 8, 2024
@jamesrb1 jamesrb1 deleted the james/alternate-fix-for-clobbered-purchase-handler branch August 8, 2024 16:36
joshdholtz added a commit that referenced this pull request Aug 22, 2024
**This is an automatic release.**

### New Features
* Price rounding logic (#4132) via James Borthwick (@jamesrb1)
### Bugfixes
* [Customer Center] Migrate to List style (#4190) via Cody Kerns
(@codykerns)
* [Paywalls] Improve locale consistency (#4158) via Josh Holtz
(@joshdholtz)
* Set Paywalls Tester deployment target to iOS 15 (#4196) via James
Borthwick (@jamesrb1)
* [Customer Center] Hide Contact Support button if URL can't be created
(#4192) via Cesar de la Vega (@vegaro)
* Fix the setting for SKIP_INSTALL in Xcode project (#4195) via Andy
Boedo (@aboedo)
* [Customer Center] Improving customer center buttons (#4165) via Cody
Kerns (@codykerns)
* Revert workaround for iOS 18 beta 5 SwiftUI crash (#4173) via Mark
Villacampa (@MarkVillacampa)
* [Paywalls] Make iOS version calculation lazy (#4163) via Mark
Villacampa (@MarkVillacampa)
* [Paywalls] Observe `PurchaseHandler` when owned externally (#4097) via
James Borthwick (@jamesrb1)
### Other Changes
* [Customer Center] Clean up colors in WrongPlatformView and
NoSubscriptionsView (#4204) via Cesar de la Vega (@vegaro)
* Fix failing `all-tests` and retry more flaky tests (#4188) via Josh
Holtz (@joshdholtz)
* Compatibility content unavailable improvements (#4197) via James
Borthwick (@jamesrb1)
* Create lane to enable customer center (#4191) via Cesar de la Vega
(@vegaro)
* XCFramework artifacts in CircleCI (#4189) via Andy Boedo (@aboedo)
* [Customer Center] CustomerCenterViewModel checks whether the app is
the latest version (#4169) via JayShortway (@JayShortway)
* export RevenueCatUI xcframework (#4172) via Andy Boedo (@aboedo)
* Corrects references from ManageSubscriptionsButtonStyle to
ButtonsStyle. (#4186) via JayShortway (@JayShortway)
* Speed up carthage installation tests (#4184) via Andy Boedo (@aboedo)
* Customer center improvements (#4166) via James Borthwick (@jamesrb1)
* replace `color(from colorInformation:)` global with extension (#4183)
via Andy Boedo (@aboedo)
* Fix tests in main (#4174) via Andy Boedo (@aboedo)
* Enable customer center tests (#4171) via James Borthwick (@jamesrb1)
* [Customer Center] Initial implementation (#3967) via Cesar de la Vega
(@vegaro)

---------

Co-authored-by: Josh Holtz <me@joshholtz.com>
nyeu pushed a commit that referenced this pull request Oct 2, 2024
Josh kindly [fixed a
problem](#4035) while I
was away where the purchase handler would get clobbered when the paywall
was re-rendered. This PR restores avoiding having one struct owned by
two @StateObjects at the same time.
nyeu pushed a commit that referenced this pull request Oct 2, 2024
**This is an automatic release.**

### New Features
* Price rounding logic (#4132) via James Borthwick (@jamesrb1)
### Bugfixes
* [Customer Center] Migrate to List style (#4190) via Cody Kerns
(@codykerns)
* [Paywalls] Improve locale consistency (#4158) via Josh Holtz
(@joshdholtz)
* Set Paywalls Tester deployment target to iOS 15 (#4196) via James
Borthwick (@jamesrb1)
* [Customer Center] Hide Contact Support button if URL can't be created
(#4192) via Cesar de la Vega (@vegaro)
* Fix the setting for SKIP_INSTALL in Xcode project (#4195) via Andy
Boedo (@aboedo)
* [Customer Center] Improving customer center buttons (#4165) via Cody
Kerns (@codykerns)
* Revert workaround for iOS 18 beta 5 SwiftUI crash (#4173) via Mark
Villacampa (@MarkVillacampa)
* [Paywalls] Make iOS version calculation lazy (#4163) via Mark
Villacampa (@MarkVillacampa)
* [Paywalls] Observe `PurchaseHandler` when owned externally (#4097) via
James Borthwick (@jamesrb1)
### Other Changes
* [Customer Center] Clean up colors in WrongPlatformView and
NoSubscriptionsView (#4204) via Cesar de la Vega (@vegaro)
* Fix failing `all-tests` and retry more flaky tests (#4188) via Josh
Holtz (@joshdholtz)
* Compatibility content unavailable improvements (#4197) via James
Borthwick (@jamesrb1)
* Create lane to enable customer center (#4191) via Cesar de la Vega
(@vegaro)
* XCFramework artifacts in CircleCI (#4189) via Andy Boedo (@aboedo)
* [Customer Center] CustomerCenterViewModel checks whether the app is
the latest version (#4169) via JayShortway (@JayShortway)
* export RevenueCatUI xcframework (#4172) via Andy Boedo (@aboedo)
* Corrects references from ManageSubscriptionsButtonStyle to
ButtonsStyle. (#4186) via JayShortway (@JayShortway)
* Speed up carthage installation tests (#4184) via Andy Boedo (@aboedo)
* Customer center improvements (#4166) via James Borthwick (@jamesrb1)
* replace `color(from colorInformation:)` global with extension (#4183)
via Andy Boedo (@aboedo)
* Fix tests in main (#4174) via Andy Boedo (@aboedo)
* Enable customer center tests (#4171) via James Borthwick (@jamesrb1)
* [Customer Center] Initial implementation (#3967) via Cesar de la Vega
(@vegaro)

---------

Co-authored-by: Josh Holtz <me@joshholtz.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants