-
Notifications
You must be signed in to change notification settings - Fork 55
refactor: track-change use ZenObservable over RxJS #1377
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
refactor: track-change use ZenObservable over RxJS #1377
Conversation
…tude-TypeScript into no-ticket-zen-observable-dead-click
…tude-TypeScript into no-ticket-zen-observable-dead-click
…tude-TypeScript into no-ticket-zen-observable-dead-click
Co-authored-by: Copilot <[email protected]>
…ack-dead-click.test.ts Co-authored-by: Copilot <[email protected]>
…ack-dead-click.test.ts Co-authored-by: Copilot <[email protected]>
|
bugbot run |
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.
✅ Bugbot reviewed your changes and found no bugs!
Comment @cursor review or bugbot run to trigger another review on this PR
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.
Pull Request Overview
This PR migrates the change event observable from RxJS to Zen Observable implementation to align with the project's move away from RxJS dependencies for change event tracking.
- Replaced RxJS
fromEventwith manual ZenObservable implementation for change events - Converted RxJS pipe-based operators (
.pipe(filter(), map())) to ZenObservable instance methods (.filter().map()) - Updated type definitions to use
ZenObservableinstead of RxJSObservablefor change events - Exported
Unsubscribabletype from analytics-core to support the migration
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test-server/autocapture/element-interactions.html | Enabled element interactions and disabled remote config for testing |
| packages/plugin-autocapture-browser/src/autocapture/track-change.ts | Migrated from RxJS pipe operators to ZenObservable instance methods |
| packages/plugin-autocapture-browser/src/autocapture-plugin.ts | Replaced RxJS fromEvent with manual ZenObservable implementation for change events and updated type definitions |
| packages/analytics-core/src/utils/observable.ts | Exported Unsubscribable type |
| packages/analytics-core/src/index.ts | Added Unsubscribable to public exports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/plugin-autocapture-browser/src/autocapture/track-change.ts
Outdated
Show resolved
Hide resolved
…tude-TypeScript into AMP-142598-refactor-track-change
Mercy811
left a comment
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.
LGTM!
02270be
into
zen-observable-migration
Summary
Checklist
Note
Replaces RxJS-based change tracking with ZenObservable + multicast, updates types/subscriptions, and exports Unsubscribable; test page config toggled.
fromEvent(...).pipe(...)change tracking with Zen Observable viamulticast(new ZenObservable(...))usinggetGlobalScopefor DOM listeners inautocapture-plugin.ts.AllWindowObservables.ChangeObservabletoZenObservableand subscriptions toUnsubscribable.track-change.tsto use Zen Observable methods (filter,map) instead of RxJS operators.Unsubscribablefromutils/observableand re-export inindex.ts.fetchRemoteConfigdisabled; enableautocapture.elementInteractionsinelement-interactions.html.Written by Cursor Bugbot for commit af190e5. Configure here.