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

Support on feature flags loaded #111

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nrobi144
Copy link

@nrobi144 nrobi144 commented Jun 3, 2024

💡 Motivation and Context

Closes #81

💚 How did you test it?

⚠️ Requires testing after initial feedback

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs and example project if needed.
  • No breaking change or entry added to the changelog.

@nrobi144 nrobi144 requested a review from marandaneto as a code owner June 3, 2024 07:12
@@ -53,6 +59,9 @@ class PosthogFlutterPlugin : FlutterPlugin, MethodCallHandler {
debug = enableDebug
sdkName = "posthog-flutter"
sdkVersion = postHogVersion
onFeatureFlags = PostHogOnFeatureFlags {
Copy link
Member

Choose a reason for hiding this comment

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

Using the callback is a good idea but it'd not work in some cases, for example
https://github.com/PostHog/posthog-android/blob/8013cccadfe52ad347eedcb492ce01690a1f7786/posthog/src/main/java/com/posthog/internal/PostHogFeatureFlags.kt#L40-L45
in this case, the callback will never be called, we'd need to either change the PostHogOnFeatureFlags interface which is a breaking change or find a way to expose the loaded flags differently (on the iOS and Android SDK)

@@ -29,6 +33,8 @@ class PosthogFlutterPlugin : FlutterPlugin, MethodCallHandler {
channel.setMethodCallHandler(this)
}

private val isLoaded = MutableStateFlow(false);
Copy link
Member

Choose a reason for hiding this comment

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

I'd not use coroutines but rather implement a dependency-free mechanism, there are a few days to do this.

Copy link
Author

Choose a reason for hiding this comment

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

makes sense

@@ -44,6 +47,9 @@ public class PosthogFlutterPlugin: NSObject, FlutterPlugin {
postHogSdkName = "posthog-flutter"
postHogVersion = postHogFlutterVersion

NotificationCenter.default.addObserver(forName: PostHogSDK.didReceiveFeatureFlags, object: nil, queue: nil) { (notification) in
Copy link
Member

Choose a reason for hiding this comment

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

Similar to https://github.com/PostHog/posthog-flutter/pull/111/files#r1624601103
but in this case didReceiveFeatureFlags is called even if there's an error, we'd probably need something more so we know if feature flags are errored or not.

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.

Support onFeatureFlags callback
2 participants