-
Notifications
You must be signed in to change notification settings - Fork 576
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
Implement App/User Listener #5080
Conversation
Used the `subscribe` and `unsubscribe` methods to finish the app listener implementation.
private listeners = new Listeners<AppChangeCallback, AppListenerToken, [binding.App]>({ | ||
register(callback: () => void, internal: binding.App): AppListenerToken { | ||
const token = internal.subscribe(callback); | ||
return { internal, token }; |
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.
Sorry for the confusion on this when we went through it on the call. It shouldn't be needed to include the App internal into the token, since you can simple read it off the SDK App
instance using this.internal
.
private listeners = new Listeners<AppChangeCallback, AppListenerToken, [binding.App]>({ | |
register(callback: () => void, internal: binding.App): AppListenerToken { | |
const token = internal.subscribe(callback); | |
return { internal, token }; | |
private listeners = new Listeners<AppChangeCallback, binding.AppSubscriptionToken>({ | |
register(callback: () => void): binding.AppSubscriptionToken { | |
return this.internal.subscribe(callback); |
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.
This is fixed now. Needed to convert this into an arrow function so that the App
context was binded to this
.
@@ -68,6 +74,16 @@ export class User< | |||
/** @internal */ | |||
public internal: binding.SyncUser; | |||
|
|||
private listeners = new Listeners<UserChangeCallback, UserListenerToken, [binding.SyncUser]>({ |
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.
The same as with the App
.. you should be able to get the relevant binding.SyncUser
via this.internal
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.
Also fixed
846d646
to
014ac0e
Compare
* Finish App Listener Used the `subscribe` and `unsubscribe` methods to finish the app listener implementation. * Implement user listeners
* Finish App Listener Used the `subscribe` and `unsubscribe` methods to finish the app listener implementation. * Implement user listeners
* Finish App Listener Used the `subscribe` and `unsubscribe` methods to finish the app listener implementation. * Implement user listeners
* Finish App Listener Used the `subscribe` and `unsubscribe` methods to finish the app listener implementation. * Implement user listeners
* Finish App Listener Used the `subscribe` and `unsubscribe` methods to finish the app listener implementation. * Implement user listeners
What, How & Why?
Used the
subscribe
andunsubscribe
methods to finish the app and user listener implementation.This closes #5062
☑️ ToDos
Compatibility
label is updated or copied from previous entryCOMPATIBILITY.md
package.json
s (if updating internal packages)Breaking
label has been applied or is not necessaryIf this PR adds or changes public API's: