Hel 4506 update ios embedded paywall#91
Conversation
…ther safety fixes.
📝 WalkthroughWalkthroughThis PR requires callers to provide a replacement widget when an embedded paywall is not shown, updates availability checks to combine capability and paywall-targeting info, changes iOS native view construction from an upsell helper to direct paywall creation, and adapts call sites/tests and docs accordingly. ChangesEmbedded Paywall Availability & Presentation
Sequence DiagramsequenceDiagram
participant App as Flutter App
participant MC as MethodChannel
participant AC as AvailabilityChecker
participant PW as NativePaywall
participant UI as Flutter UI
App->>MC: getUpsellWidget(trigger, paywallNotShownReplacement)
MC->>AC: _checkEmbeddedAvailability(trigger)
AC->>AC: canPresentUpsell(trigger)
alt canShow == true
AC->>AC: getPaywallInfo(trigger)
alt shouldShow == true
AC-->>MC: canShow: true
else shouldShow == false
AC-->>MC: canShow: false (reason: targetingHoldout)
end
else
AC-->>MC: canShow: false
end
alt canShow == true
MC->>PW: instantiate paywall view (trigger, traits)
PW-->>UI: platform-hosted paywall widget
else
MC-->>UI: paywallNotShownReplacement
end
UI-->>App: rendered result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/helium_stripe/test/helium_stripe_test.dart (1)
112-118:⚠️ Potential issue | 🔴 CriticalMockHeliumFlutterPlatform is missing two required method implementations — this will cause compile errors.
The mock is incomplete and lacks implementations for
handleURLandsetThirdPartyAnalyticsAnonymousId, both of which are declared in theHeliumFlutterPlatforminterface. Since the mock class implements this interface, all abstract methods must be present. Add these two methods with appropriate stub implementations (e.g., returningFuture.value(false)forhandleURLandFuture.value()forsetThirdPartyAnalyticsAnonymousId).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/helium_stripe/test/helium_stripe_test.dart` around lines 112 - 118, MockHeliumFlutterPlatform is missing required implementations for the HeliumFlutterPlatform interface; add stub async methods named handleURL(String url) and setThirdPartyAnalyticsAnonymousId(String id) to the MockHeliumFlutterPlatform class so the class implements the interface fully — implement handleURL to return Future.value(false) (or appropriate boolean) and setThirdPartyAnalyticsAnonymousId to return Future.value() (void) or a completed Future to satisfy the interface signature; ensure method signatures match HeliumFlutterPlatform exactly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/helium_flutter/ios/helium_flutter/Sources/helium_flutter/FLNativeViewFactory.swift`:
- Around line 56-70: makePaywallView currently returns hostingController.view
only, so the local UIHostingController is deallocated immediately which breaks
SwiftUI state; retain the controller as a stored property on FLNativeView (e.g.,
add a property like paywallHostingController: UIHostingController<...>? on the
FLNativeView class), assign hostingController to that property before returning
the view in makePaywallView, and if appropriate make the hosting controller a
child view controller (call addChild(hostingController) and
hostingController.didMove(toParent:)) or otherwise ensure the controller's
lifetime matches the view's so SwiftUI state continues to update.
---
Outside diff comments:
In `@packages/helium_stripe/test/helium_stripe_test.dart`:
- Around line 112-118: MockHeliumFlutterPlatform is missing required
implementations for the HeliumFlutterPlatform interface; add stub async methods
named handleURL(String url) and setThirdPartyAnalyticsAnonymousId(String id) to
the MockHeliumFlutterPlatform class so the class implements the interface fully
— implement handleURL to return Future.value(false) (or appropriate boolean) and
setThirdPartyAnalyticsAnonymousId to return Future.value() (void) or a completed
Future to satisfy the interface signature; ensure method signatures match
HeliumFlutterPlatform exactly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 749b712c-b42d-45ee-ba70-9bdbc0c81d84
📒 Files selected for processing (8)
example/lib/presentation/view_for_trigger_page.dartpackages/helium_flutter/ios/helium_flutter/Sources/helium_flutter/FLNativeViewFactory.swiftpackages/helium_flutter/ios/helium_flutter/Sources/helium_flutter/HeliumFlutterPlugin.swiftpackages/helium_flutter/lib/core/helium_flutter_method_channel.dartpackages/helium_flutter/lib/core/helium_flutter_platform.dartpackages/helium_flutter/lib/helium_flutter.dartpackages/helium_flutter/test/helium_flutter_test.dartpackages/helium_stripe/test/helium_stripe_test.dart
| func makePaywallView(trigger: String, customPaywallTraits: HeliumUserTraits?) -> UIView { | ||
| let config = PaywallPresentationConfig(customPaywallTraits: customPaywallTraits) | ||
| let paywallView = HeliumPaywall( | ||
| trigger: trigger, | ||
| config: config, | ||
| eventHandlers: PaywallEventHandlers.withHandlers( | ||
| onAnyEvent: postPaywallEvent | ||
| ) | ||
| ) | ||
| guard let swiftUIView else { | ||
| // this should never happen because we check canPresentUpsell before creating this view | ||
| return UITextField() | ||
| ) { _ in | ||
| EmptyView() | ||
| } | ||
| let hostingController = UIHostingController(rootView: swiftUIView) | ||
| let hostingController = UIHostingController(rootView: paywallView) | ||
| hostingController.view.backgroundColor = .clear | ||
| return hostingController.view | ||
| } |
There was a problem hiding this comment.
UIHostingController is not retained and will be deallocated immediately — SwiftUI state management breaks.
makePaywallView returns only hostingController.view; the local hostingController variable goes out of scope and ARC releases it as soon as the function returns. A UIView does not retain its UIViewController, so the UIHostingController is gone before the paywall ever appears on screen.
UIHostingController drives SwiftUI's entire lifecycle management. Once it is deallocated "it can't perform its job", and the pattern of only keeping its view "can even appear to work fine until it doesn't" — the issue being that "we don't retain UIHostingController after the hosting setup, and it is deallocated instantly."
It is "tempting to embed [the SwiftUI view] inside UIHostingController and then simply plug out the root view of the controller, ignoring UIHostingController completely", but "there are scenarios where it can hit you hard." The recommendation is to "always store it somewhere so that its lifetime matches the lifetime of the view it embeds" — either as a child view controller or "at least save it in the property if you create it inside another UIView."
For an interactive HeliumPaywall (purchase taps, loading states, async event callbacks back through PaywallEventHandlers), SwiftUI state updates will silently stop propagating once the hosting controller is gone.
Fix: retain the hosting controller as a stored property on FLNativeView:
🔒 Proposed fix — retain the `UIHostingController`
class FLNativeView: NSObject, FlutterPlatformView {
let arguments: Any?
+ private var hostingController: UIViewController?
+
private lazy var _view: UIView = {
let args = arguments as? [String: Any] ?? [:]
let trigger = args["trigger"] as? String ?? ""
let traitsMap = convertMarkersToBooleans(args["customPaywallTraits"] as? [String: Any])
let customPaywallTraits = traitsMap.map { HeliumUserTraits($0) }
- return makePaywallView(trigger: trigger, customPaywallTraits: customPaywallTraits)
+ let config = PaywallPresentationConfig(customPaywallTraits: customPaywallTraits)
+ let paywallView = HeliumPaywall(
+ trigger: trigger,
+ config: config,
+ eventHandlers: PaywallEventHandlers.withHandlers(
+ onAnyEvent: postPaywallEvent
+ )
+ ) { _ in
+ EmptyView()
+ }
+ let hc = UIHostingController(rootView: paywallView)
+ hc.view.backgroundColor = .clear
+ hostingController = hc // ← keep it alive
+ return hc.view
}()
// ... init unchanged ...
- func makePaywallView(trigger: String, customPaywallTraits: HeliumUserTraits?) -> UIView {
- let config = PaywallPresentationConfig(customPaywallTraits: customPaywallTraits)
- let paywallView = HeliumPaywall(
- trigger: trigger,
- config: config,
- eventHandlers: PaywallEventHandlers.withHandlers(
- onAnyEvent: postPaywallEvent
- )
- ) { _ in
- EmptyView()
- }
- let hostingController = UIHostingController(rootView: paywallView)
- hostingController.view.backgroundColor = .clear
- return hostingController.view
- }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/helium_flutter/ios/helium_flutter/Sources/helium_flutter/FLNativeViewFactory.swift`
around lines 56 - 70, makePaywallView currently returns hostingController.view
only, so the local UIHostingController is deallocated immediately which breaks
SwiftUI state; retain the controller as a stored property on FLNativeView (e.g.,
add a property like paywallHostingController: UIHostingController<...>? on the
FLNativeView class), assign hostingController to that property before returning
the view in makePaywallView, and if appropriate make the hosting controller a
child view controller (call addChild(hostingController) and
hostingController.didMove(toParent:)) or otherwise ensure the controller's
lifetime matches the view's so SwiftUI state continues to update.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/helium_flutter/lib/core/helium_flutter_method_channel.dart`:
- Around line 891-898: The code currently calls getPaywallInfo(trigger) and only
blocks showing when info != null && !info.shouldShow, which lets a null info
fall through and potentially return a positive CanPresentUpsellResult; change
the logic in the method around getPaywallInfo(trigger) so that if info is null
you treat it as failing the "shouldShow" check and return
CanPresentUpsellResult.fromMap({'canShow': false, 'paywallUnavailableReason':
'paywallInfoMissing'}) (or use 'targetingHoldout' if preferred), ensuring the
final returned result enforces the "canPresentUpsell && shouldShow" contract
rather than returning the original result when paywall info is missing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f1da8598-3195-4460-92bd-c57b11904a09
📒 Files selected for processing (6)
example/lib/presentation/view_for_trigger_page.dartpackages/helium_flutter/lib/core/helium_flutter_method_channel.dartpackages/helium_flutter/lib/core/helium_flutter_platform.dartpackages/helium_flutter/lib/helium_flutter.dartpackages/helium_flutter/test/helium_flutter_test.dartpackages/helium_stripe/test/helium_stripe_test.dart
|
Promptless prepared a documentation update related to this change. Triggered by PR #91 in helium_flutter Updated embedded paywall docs to reflect the new required |
1 participant
Note
Medium Risk
Medium risk because it changes the public Flutter API for
getUpsellWidget(new required parameter) and alters embedded paywall show/no-show logic across Dart + iOS native bridging, which could affect when paywalls render or fall back.Overview
Embedded paywall API is updated:
getUpsellWidgetnow requires apaywallNotShownReplacementwidget, and the wrapper renders this replacement when an embedded paywall can’t be shown (instead of a default fallback widget).Availability checks are tightened for embedded paywalls by combining
canPresentUpsellwithgetPaywallInfo.shouldShowto treat targeting holdouts as non-displayable and return explicit unavailability reasons.iOS embedded paywall creation is refactored to build a
HeliumPaywallwithPaywallPresentationConfig(customPaywallTraits:), decoding boolean marker values and forwardingcustomPaywallTraitsfrom Flutter. Boolean-marker decoding helpers are moved to file-level Swift functions, and Dart method-channel calls forgetPaywallInfo/canPresentUpsellnow handle exceptions and log failures more consistently.Reviewed by Cursor Bugbot for commit 52dcbca. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Breaking Changes