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

wasm-ext: WebTransport #3825

Closed
mriise opened this issue Apr 24, 2023 · 7 comments · Fixed by #4015
Closed

wasm-ext: WebTransport #3825

mriise opened this issue Apr 24, 2023 · 7 comments · Fixed by #4015

Comments

@mriise
Copy link

mriise commented Apr 24, 2023

Description

Add WebTransport support for wasm targets. Similar to the discussion in #3611, this should be done with the web-sys crate.

Requirements

  1. Get a new version of web-sys published (Bump versions rustwasm/wasm-bindgen#3400), the bindings were added a while ago in Add WebTransport webidl to unstable rustwasm/wasm-bindgen#3344 but haven't been published yet.
  2. Add a new transport crate (libp2p-websys-webtransport?) and implementation.
  3. Test compatibility with go-libp2p's webtransport implementation.

Open questions

  • WebTransport is an unstable api for web-sys, do we pass this into build script or just add as note and have users pass the flag?

Are you planning to do it yourself in a pull request?

Yes

@mriise
Copy link
Author

mriise commented Apr 24, 2023

Unfortunately I am blocked doing any implementation until the web-sys version update as rust will scream violently that you cant link 2 different versions of wasm-bindgen-shared in the same package. In theory the workaround is to patch everything (this includes forking getrandom to patch) but that's a bit much...

Will do so if the ver bump takes too long though...

@thomaseizinger
Copy link
Contributor

Would love to see libp2p-websys-webtransport!

The linked PR is not a breaking change, meaning you should be able to (temporarily) use [patch.crates-io] in our workspace manifest to redirect wasm-bindgen-shared to a Git dependency.

That at least allows us to see a passing CI apart from the cargo deny check that will flag your Git dependency :)

@thomaseizinger
Copy link
Contributor

WebTransport is an unstable api for web-sys, do we pass this into build script or just add as note and have users pass the flag?

We can't make that decision for our users. If they have web-sys in their dependency tree, us activating that flag could result in breaking changes for them. I'd suggest that we release it as an alpha and not expose it via the meta-crate, similar to how we are currently dealing with webrtc and quic.

Users then need to depend in it separately and we can document that they need to set this special cfg for things to work.

@mriise
Copy link
Author

mriise commented Apr 24, 2023

The linked PR is not a breaking change, meaning you should be able to (temporarily) use [patch.crates-io] in our workspace manifest to redirect wasm-bindgen-shared to a Git dependency.

Figured out I was putting the patch in the local crate level when it needed to be at the workspace level. Things are building now :)

@thomaseizinger
Copy link
Contributor

The linked PR is not a breaking change, meaning you should be able to (temporarily) use [patch.crates-io] in our workspace manifest to redirect wasm-bindgen-shared to a Git dependency.

Figured out I was putting the patch in the local crate level when it needed to be at the workspace level. Things are building now :)

Neat! Feel free to open a draft PR.

@Jorropo
Copy link

Jorropo commented May 22, 2023

Somewhat related, I got a PoC of this working in go-libp2p libp2p/go-libp2p#2191 this was way easier than I expected and took half a day (the stream api maps fairly well maps to go's io.ReadWriteCloser).
I might try the same here if no one pickups this just to get an excuse to write some rust.

@mxinden
Copy link
Member

mxinden commented May 25, 2023

@Jorropo in case you want to work on this, you can build on top of #3846.

@mergify mergify bot closed this as completed in #4015 Jun 23, 2023
mergify bot pushed a commit that referenced this issue Jun 23, 2023
This PR implements `Transport` for WebTransport for browsers by using web-sys.

Related: #3846.
Resolves: #3825.

Pull-Request: #4015.


  
Co-Authored-By: Yiannis Marangos <[email protected]>
  

  
Co-Authored-By: Maciej Zwoliński <[email protected]>
  

  
Co-Authored-By: Yiannis Marangos <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants