-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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 WebSocket API #7051
Implement WebSocket API #7051
Conversation
Currently all tests related to connecting will fail because there are leaking async ops |
|
interface CloseEvent extends Event { | ||
/** | ||
* Returns the WebSocket connection close code provided by the server. | ||
*/ |
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.
nit: These could probably be made into single line doc comments
nevermind - I think these were copied directly from dom.d.ts
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.
yea they were
this.origin = eventInitDict?.origin ?? ""; | ||
this.lastEventId = eventInitDict?.lastEventId ?? ""; | ||
} | ||
} |
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.
Does it make sense to move the MessageEvent in a separate PR?
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.
i could if you want, but i dont see all that much point to it
promise.resolve(); | ||
}; | ||
await promise; | ||
}); |
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.
Very nice - thanks @lucacasonato for ridding us of openssl
This reverts commit ed2c19f.
Is this patch ready to land? |
# Conflicts: # cli/Cargo.toml # cli/build.rs
@ry yes |
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.
LGTM - huge contribution @crowlKats - many thanks!
Closes #3539.
Currently most tests dont work. Not sure how to run the websocket server simulatenously. some borrowmuterrors in some places and possible case of missing RID in eventloop when closing.