-
Notifications
You must be signed in to change notification settings - Fork 27
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
chore: WebViewSyncPlugin #253
base: main
Are you sure you want to change the base?
Conversation
Thanks @crleona for better supporting web views.
I don't see why customers should turn session tracking on in webviews as it's part of the mobile app. The session should start when they open the mobile app instead of open webviews. Customers should turn browser SDK session tracking off. |
@Mercy811 - |
|
||
private let webviews = NSHashTable<WKWebView>.weakObjects() | ||
|
||
private static let swizzleWebViewInitializer: Void = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of swizzling the initializer which would affect all web views, would it be better to document that consumers could attach WebViewSyncPlugin.registerWebview(webview)
at their own callsites to attach the Amplitude plugin?
This would allow us to specify if we wish to attach the user script for a particular webview rather than automatically do it for all, as well as avoid any downsides of the swizzling approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @thedavidharris, this is just a sample plugin that we do not expect to ship with the SDK, I'd encourage you to modify it to fit your specific needs.
Summary
Adds a prototype plugin to sync user, device, and session ids to all webviews in an app. This may be an easier approach for hybrid apps to sync Amplitude properties to their webviews, vs manually injecting device and session ids.
(The extraneous session start events are generated from the JS library auto tracking - we should add a method to set the session id without generating these).
Customers that are newly adopting this sync strategy may want to change the JS plugin to compare and report mismatches in device / user / session id vs setting them directly.
Current rough JS plugin:
How it works:
Checklist