Thread store and event parsing to create threads #1828
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
germain-gg
force-pushed
the
gsouquet/threaded-messaging-2349
branch
from
e9f3d13 to
1811390
Compare
turt2live
reviewed
Aug 5, 2021
There was a problem hiding this comment.
Generally it looks sane, though a couple things: is this meant to be unstable functionality? If so, annotating the functions aggressively with unstable warnings would be appreciated (to allow for breaking changes during development / discouraging use until it's ready).
More documentation on the public fields/functions would be appreciated as well, to help with understanding and those who use this thing outside of us.
turt2live
approved these changes
Aug 25, 2021
| @@ -146,7 +146,7 @@ export class Room extends EventEmitter { | |||
| public oldState: RoomState; | |||
| public currentState: RoomState; | |||
|
|
|||
| private threads = new Set<Thread>(); | |||
| public threads = new Set<Thread>(); | |||
There was a problem hiding this comment.
it's a little dangerous exposing this publicly, but not the end of the world. Can it get some jsdoc to guide the developer on using it safely? (ie: is it safe to mutate, or should it be treated as readonly?)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a first implementation for thread in the
js-sdk. There is currently no UI associated yet but will follow shortlyGiven that the concept of thread will be completely client-side for now, references to the root and tails of the thread are kept, to be able to rebuild the reply chain upon a refresh or so that it can be synchronized between multiple clients