Make CloudConnection an EventEmitter #19
Conversation
Make CloudConnection event-based https://stackoverflow.com/a/39145058
|
I will review later today if I can! This looks really nice so I will try compilation as well. Before merging I will try to understand the code though |
Could you approve running workflows for this PR? |
src/classes/CloudConnection.ts
Outdated
| } | ||
| }); | ||
| this.connection.on("message", (e) => { | ||
| this.emit("message-may-be-empty", e); |
There was a problem hiding this comment.
Mainly for debugging.
|
Hmm: |
src/classes/CloudConnection.ts
Outdated
| this.connection.on("message", (e) => { | ||
| this.emit("message-may-be-empty", e); | ||
| if (!e) return; | ||
| this.emit("message", e); |
There was a problem hiding this comment.
For situations when the raw responses need to be handled. (e.g. additional metadata sent by custom servers)
There was a problem hiding this comment.
Custom servers don't have support right now if I remember correctly. That's something I would add in maybe 2.3.0 (this will go 2.2.0)
There was a problem hiding this comment.
webdev03: A server might send data about user joins/disconnects in some cases.
|
Don't worry about linting, I can do it after merging |
| user: this.session.sessionJSON.user.username, | ||
| project_id: this.id.toString() | ||
| }); | ||
| this.emit("connect", null); |
There was a problem hiding this comment.
So command-line applications can show UI for the connection status.
| this.connection.on("close", () => { | ||
| if (!this.disconnected) this.connect(); | ||
| if (!this.disconnected) { | ||
| this.emit("reconnect", null); |
| * Sends a packet through cloud | ||
| */ | ||
| private send(data) { | ||
| this.emit("internal-send", data); |
| * Closes the cloud connection | ||
| */ | ||
| close() { | ||
| this.emit("close", null); |
There was a problem hiding this comment.
So it's easy to implement UI for connection status.
|
Can you try just |
Okay |
Okay |
|
Why's it not running workflows? |
|
:/ |
https://github.com/webdev03/meowclient/runs/6150669995?check_suite_focus=true#step:4:43 |
|
https://www.npmjs.com/package/events |
|
Need it approved to run |
|
@webdev03 Need to make a change to |
|
I might have to change tsconfig - I will do it later Sure you can try! |
Builds will fail otherwise, so there'll be no way to know if it runs. |
Signed-Off-By: webdev03 webdev03#19 (comment) webdev03#19 (comment)
|
@webdev03 Could you approve workflows? |
src/classes/CloudConnection.ts
Outdated
|
|
||
| class CloudConnection { | ||
| class CloudConnection extends events.EventEmitter { | ||
| super(); |
|
FYI: neotest is really broken so I am going to switch to a better solution soon |
|
It compiles!! |
| session: Session; | ||
| server?: string; | ||
| }) { | ||
| super(); |
There was a problem hiding this comment.
Does it build now?
Will it get merged now? |
|
Just a few more changes, such as me adding a queue system. Now I can merge it. You are a really smart person! |
|
Just wondering, do you want to add more emitters? I removed message since it was a bit confusing - set makes more sense. |
I'm not sure. Maybe add one for notifications??? (Then it'd be easy to make a prebuilt QEMU vm that sends emails for notifications) |
2 participants
Make
CloudConnectionevent-basedhttps://stackoverflow.com/a/39145058
@webdev03 Could you test this? I'm not sure if it works.
EventEmitterinterface