Skip to content
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

Remove Client.subscribe #788

Open
hackerwins opened this issue Apr 25, 2024 · 2 comments · Fixed by #789
Open

Remove Client.subscribe #788

hackerwins opened this issue Apr 25, 2024 · 2 comments · Fixed by #789
Labels
cleanup 🧹 Paying off technical debt sdk ⚒️

Comments

@hackerwins
Copy link
Member

hackerwins commented Apr 25, 2024

Description:

The current implementation of Document.subscribe provides sufficient event support corresponding to Channel or Room. To simplify the usage of the Client as a simple Connection object, the Client.subscribe method should be removed.

Why:

Removing Client.subscribe, the system can be streamlined for better usability and consistent functionality.

@hackerwins hackerwins added cleanup 🧹 Paying off technical debt sdk ⚒️ labels Apr 25, 2024
@chacha912
Copy link
Contributor

The current ClientEvent includes four events: StatusChangedEvent, DocumentChangedEvent, StreamConnectionStatusChangedEvent, and DocumentSyncedEvent. Upon reconsideration, it appears some places may still require these events. What do you think about the following modifications?

  • StatusChangedEvent = activated | deactivated
    • Since it's not being used, it can be removed for now and added back when needed.
  • DocumentChangedEvent
    • This event can be used to detect changes in the document when performing manual sync. It can be moved to document.subscribe().
  • StreamConnectionStatusChangedEvent = connected | disconnected
    • Since the watch stream is established for each document, it can be moved to document.subscribe().
  • DocumentSyncedEvent = synced | sync-failed
    • It can be moved to document.subscribe().

@hackerwins
Copy link
Member Author

Thank you for the detailed investigation. Since there is currently no use case for Client.Subscribe, it may be a good idea to remove it and add it later as needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup 🧹 Paying off technical debt sdk ⚒️
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants