feat(unstable): Implement QUIC #21942
Conversation
devsnek
force-pushed
the
quic
branch
2 times, most recently
from
8d2f39b to
84747e1
Compare
devsnek
force-pushed
the
quic
branch
4 times, most recently
from
28254df to
f7eb83e
Compare
devsnek
force-pushed
the
quic
branch
4 times, most recently
from
37df306 to
aec0bb8
Compare
devsnek
force-pushed
the
quic
branch
4 times, most recently
from
e208a10 to
6d9a15f
Compare
There was a problem hiding this comment.
This looks pretty good. I agree with @littledivy's comments re: ring.
I think we should consider merging this into ext/net rather than creating a new module, however.
devsnek
force-pushed
the
quic
branch
3 times, most recently
from
d05ef59 to
823a8f9
Compare
There was a problem hiding this comment.
@devsnek this is fantastic work, I'm impressed how few lines of code you needed to implement that 💪
devsnek
force-pushed
the
quic
branch
4 times, most recently
from
a5b1091 to
7645454
Compare
devsnek
force-pushed
the
quic
branch
2 times, most recently
from
b7fdfb6 to
001fb6a
Compare
devsnek
force-pushed
the
quic
branch
2 times, most recently
from
4d4f24e to
b21790d
Compare
|
How hard or how long would it take to implement Webtransport with this PR ? My suggestion is : why not adding it at the same time before merging in 1.41 since WT is 76% available on the browsers. Adding a client seems "easy" enough, and a server which is upgrading the QUIC transport connection to a server (somewhat like Websocket works at the moment) |
devsnek
force-pushed
the
quic
branch
3 times, most recently
from
8638073 to
c8e070f
Compare
Probably should be done in a separate PR imo. I'm really excited about this! Is there any consideration for this in deno 1.43 @bartlomieju? |
There was a problem hiding this comment.
Very nicely done. Most of my suggestions are little nits. However, same as @mmastrac, I question whether this should be done as an extra module rather than within ext/net.
Either way, can you please resolve the merge conflicts?
runtime/js/90_deno_ns.js
Outdated
| connectQuic: quic.connectQuic, | ||
| listenQuic: quic.listenQuic, | ||
| QuicBidirectionalStream: quic.QuicBidirectionalStream, | ||
| QuicConn: quic.QuicConn, | ||
| QuicListener: quic.QuicListener, | ||
| QuicReceiveStream: quic.QuicReceiveStream, | ||
| QuicSendStream: quic.QuicSendStream, | ||
| }; | ||
|
|
There was a problem hiding this comment.
We can either include these classes in the Deno.* namespace or make the constructors of these classes illegal. The former adds to the API surface area, but the latter enables such objects to be identified with the use of instanceof. Which should we do? @bartlomieju
I also think we should merge it into ext/net, and if we want to make protocol that depends on QUIC then we can make module ( i.e WebTransport and Media over Quic (MoQ).. |
devsnek
force-pushed
the
quic
branch
2 times, most recently
from
4fe503b to
e2156ca
Compare
|
Any updates on this? @devsnek @bartlomieju |
devsnek
force-pushed
the
quic
branch
3 times, most recently
from
02bc6fb to
8e2726e
Compare
| "connectQuic", | ||
| "listenQuic", | ||
| "QuicBidirectionalStream", | ||
| "QuicConn", | ||
| "QuicListener", | ||
| "QuicReceiveStream", | ||
| "QuicSendStream", |
There was a problem hiding this comment.
Nitpick - add these to runtime/fmt_errors.rs get_suggestions_for_terminal_errors for better DX?
ext/net/lib.deno_net.d.ts
Outdated
| /** @category Network */ | ||
| export interface QuicTransportOptions { |
There was a problem hiding this comment.
All these typings are missing the **UNSTABLE**: New API, yet to be vetted. banners and @experimental markers. @crowlKats can double check if they are marked properly
Implements a QUIC interface, loosely based on the WebTransport API (a future change could add the WebTransport API, built on top of this one).
quinn is used for the underlying QUIC implementation, for a few reasons: