feat: add onFeatureFlags callback #183
Conversation
|
@ioannisj would you like to review and test this one as part of owning a bit more of this SDK? happy to do a final review as well |
There was a problem hiding this comment.
Left a few comments — overall this looks good! My main concern is around using a shared callback across platforms, where some parameters are intentionally passed empty on mobile. Not ideal in terms from a dx perspective, but can't think of another alternative either
@marandaneto leaving the final review to you
android/src/main/kotlin/com/posthog/flutter/PosthogFlutterPlugin.kt
Outdated
Show resolved
Hide resolved
android/src/main/kotlin/com/posthog/flutter/PosthogFlutterPlugin.kt
Outdated
Show resolved
Hide resolved
android/src/main/kotlin/com/posthog/flutter/PosthogFlutterPlugin.kt
Outdated
Show resolved
Hide resolved
android/src/main/kotlin/com/posthog/flutter/PosthogFlutterPlugin.kt
Outdated
Show resolved
Hide resolved
|
Oh I see this is similar to the Web implementation https://posthog.com/docs/libraries/js/features#ensuring-flags-are-loaded-before-usage hence the |
|
can we do this for the Web implementation as well? |
Co-authored-by: Ioannis J <yiannis@posthog.com>
Android try-catch and arguments (Comments 1 & 5):
The Android native code in PosthogFlutterPlugin.kt now sends an empty map for the onFeatureFlagsCallback. The try-catch block remains to handle potential errors during the invokeMethod call itself but no longer sends different argument structures.
Callback signature/location and parameter handling (Comments 2, 7, 8):
The OnFeatureFlagsCallback is now defined in PostHogConfig and passed during Posthog().setup().
The Dart callback signature void Function(List<String> flags, Map<String, dynamic> flagVariants, {bool? errorsLoading}) is maintained.
For mobile (Android/iOS), this callback will be invoked with an empty list for flags, an empty map for flagVariants, and errorsLoading will be null (if the native call is successful) or true (if a Dart-side processing error occurs in _handleMethodCall). This aligns with the native SDKs not providing these details directly in their "flags loaded" callbacks.
For web, the callback will receive the actual flags, flagVariants from posthog-js, and errorsLoading will be false (as the JS callback firing implies success).
Dart redundant null check (Comment 3):
The redundant null check in lib/src/posthog_flutter_io.dart's _handleMethodCall has been removed.
Android dataMode (Comment 4):
The unused dataMode configuration has been removed from the Android setupPostHog method in PosthogFlutterPlugin.kt.
Android main thread invocation (Comment 6):
The mainHandler.post for channel.invokeMethod in Android is kept, as it's good practice for Flutter platform channel calls.
All relevant Dart files, the Android Kotlin plugin file, and their corresponding test files have been updated to reflect these changes.
This reverts commit 8125e29.
This reverts commit 91cf32f.
|
@laf-rge Apologies for the delay here! Still willing to work on this? If so, you can merge main for a final review? |
💡 Motivation and Context
This change introduces an
onFeatureFlagscallback to thePosthogFlutter SDK. The primary motivation is to provide a reliable way for developers to know when feature flags have been loaded from the PostHog server, especially on the first application launch where flags might not be immediately available. This helps prevent race conditions and ensures that feature flag checks are made against up-to-date data.This change addresses and closes issue #81.
The callback behaves as follows:
flagsandflagVariantsparameters in the callback will be empty. Developers should usePosthog().getFeatureFlag('key')orPosthog().isFeatureEnabled('key')to retrieve specific flag values after this callback is invoked. This is because the native libraries do not have public methods for retrieving all features.💚 How did you test it?
The changes were tested through several methods:
PosthogFlutterIOimplementation to cover callback registration, successful event handling (with and without data), error handling, and malformed data scenarios.posthog-flutter. The dependency inpubspec.yamlwas pointed to this forked version, and I confirmed that theonFeatureFlagscallback fires correctly once the feature flags are available from the native SDKs.channel.invokeMethodis called on the main UI thread and that it signals readiness without attempting to send all flag data (as the native Android SDK's callback doesn't provide it directly).PostHogSDK.didReceiveFeatureFlagsand, similar to Android, signals readiness without sending all flag data due to the absence of a public API in the native iOS SDK to fetch all flags and variants at once.📝 Checklist
onFeatureFlagsand added an entry toCHANGELOG.md)