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.
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.
Sure but how do we identify subscribers then? Any ideas as to where this id can live?
There was a problem hiding this comment.
Maybe we could pass an id as the subscription parameter?
actor.subscribe(observer, {id})
There was a problem hiding this comment.
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.
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.