-
Notifications
You must be signed in to change notification settings - Fork 60
feat(background-capture): adding background capture injection to auto… #1501
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
base: main
Are you sure you want to change the base?
Conversation
Add background capture injection to
|
| } | ||
|
|
||
| // Setup background capture messenger if it is not already setup for visual tagging selector | ||
| if (window.opener && backgroundCaptureOptions.enabled && !visualTaggingOptions.messenger) { |
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.
backgroundCaptureOptions.messenger isn’t initialized when visualTagging is disabled because visualTaggingOptions.messenger always defaults to a value. Consider basing the check on !visualTaggingOptions.enabled (or whether the messengers differ) so background capture initializes when enabled.
| if (window.opener && backgroundCaptureOptions.enabled && !visualTaggingOptions.messenger) { | |
| if (window.opener && backgroundCaptureOptions.enabled && (!visualTaggingOptions.enabled || backgroundCaptureOptions.messenger !== visualTaggingOptions.messenger)) { |
🚀 Want me to fix this? Reply ex: "fix it for me".
| @@ -155,7 +167,8 @@ export class WindowMessenger implements Messenger { | |||
| this.endpoint = endpoint; | |||
| } | |||
| let amplitudeVisualTaggingSelectorInstance: any = null; | |||
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.
initialize-background-capture can race with close-background-capture, leaving the instance active after a close, and repeated initializes overwrite without closing the previous instance. Consider closing any existing instance before initializing, track a close-request flag, and skip/close the newly created instance if a close was requested while loading.
🚀 Want me to fix this? Reply ex: "fix it for me".
| } | ||
|
|
||
| // Setup background capture messenger if it is not already setup for visual tagging selector | ||
| if (window.opener && backgroundCaptureOptions.enabled && !visualTaggingOptions.messenger) { |
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.
WindowMessenger.setup adds a window message listener, but nothing removes it. If autocapturePlugin initializes more than once, duplicate listeners accumulate and leak memory. Consider adding a teardown/unsubscribe in WindowMessenger and invoking it from autocapturePlugin.teardown, or ensure the listener is added only once.
🚀 Want me to fix this? Reply ex: "fix it for me".
- @amplitude/analytics-browser@2.33.2-SR-2360.0 - @amplitude/analytics-client-common@2.4.20-SR-2360.0 - @amplitude/analytics-core@2.36.0-SR-2360.0 - @amplitude/analytics-node@1.5.30-SR-2360.0 - @amplitude/analytics-react-native@1.5.33-SR-2360.0 - @amplitude/analytics-types@2.12.0-SR-2360.0 - @amplitude/gtm-snippet@2.33.2-SR-2360.0 - @amplitude/plugin-autocapture-browser@1.19.0-SR-2360.0 - @amplitude/plugin-global-user-properties@1.2.111-SR-2360.0 - @amplitude/plugin-network-capture-browser@1.7.4-SR-2360.0 - @amplitude/plugin-page-url-enrichment-browser@0.5.10-SR-2360.0 - @amplitude/plugin-page-view-tracking-browser@2.6.7-SR-2360.0 - @amplitude/plugin-session-replay-browser@1.25.6-SR-2360.0 - @amplitude/plugin-session-replay-react-native@0.4.6-SR-2360.0 - @amplitude/plugin-web-attribution-browser@2.1.104-SR-2360.0 - @amplitude/plugin-web-vitals-browser@1.1.5-SR-2360.0 - @amplitude/segment-session-replay-plugin@0.0.0-SR-2360.0 - @amplitude/session-replay-browser@1.30.5-SR-2360.0 - @amplitude/unified@1.0.0-SR-2360.0
Summary
Checklist