-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Expose subscription as an inspection event #5391
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
🦋 Changeset detectedLatest commit: 901335e The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
errorListener?: (error: any) => void, | ||
completeListener?: () => void | ||
completeListener?: () => void, | ||
observerId?: string |
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.
I don't quite like this addition. subscribe
's signature is pseudo-standard (with Observables hopefully coming some day to the platform, see the current efforts here). I would prefer not to extend it in any custom/new way.
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.
Sure but how do we identify subscribers then? Any ideas as to where this id can live?
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.
Maybe we could pass an id as the subscription parameter?
actor.subscribe(observer, {id})
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.
cc @davidkpiano
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.
Maybe we could pass an id as the subscription parameter?
This is exactly the same thing ;p
It would be great if you could come up with a description of he exact problem you are trying to solve here and include that in the PR's description. The current description is somewhat vague. I don't really understand what subscription hierarchies are.
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.
You're right. I thought I was passing it as a property of the observer.
The problem this will solve is to identify the subscribers to an actor. A tool can use the inspect protocol and map out subscribers of every actor. This tool for instance can be used to map react component tree to actor subscriptions to visualize subscriptions at scale. Helps identify unnecessary subscriptions and places to improve. This was just an example.
Exposing subscriptions and the observer id through inspection protocol can help map out actor subscriptions. This can come handy in mapping out subscription hierarchy.