-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: migrate integration crate off-tree #1
Conversation
Signed-off-by: Enzo "raskyld" Nocera <enzo@nocera.eu>
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.
Overall this looks like a good start. Unless I'm misunderstanding there'll be quite a bit more integration code added in here
@@ -0,0 +1,60 @@ | |||
//! A first-party support of the `wasm32-wasip1` target for the **Server-Side** |
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 might have misunderstood, but I thought you were going for wasip2?
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.
Yes, it is actually targeting the Preview 2 of WASI, but I don't see mention of the Tier upgrade in the latest Rust release notes so I am not sure wasip2
toolchain is ready. I think I should ask to Bytecode Alliance fellows
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.
So atm, most people target wasip1
and use an adapter
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.
It probably isn't, that'd be a good question though. I wonder how things would go if you ran cargo component on 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.
Honestly I haven't tried it out but I would expect it to fail since you need a pretty specific toolchain (that is bundled with the Tier 2 upgrade)
I asked about that there: https://bytecodealliance.zulipchat.com/#narrow/channel/235408-rust-toolchain/topic/Promoting.20wasm32-wasip2.20to.20tier.202/near/478774747
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.
Oh Alex confirmed that! 🎉
So I can try actually! So I can try it once we release the origin PR in leptos, I will merge it as is for now but follow-up cleaning-up and production readiness PRs should arrive soon!
/// allowing you to choose between synchronous response | ||
/// (i.e. sending the whole response) and asynchronous response | ||
/// (i.e. streaming the response). | ||
pub struct Response(pub http::Response<Body>); |
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.
Nice, I like the Response type
This is a migration from leptos-rs/leptos#3063 so we have the integration crate off-tree.
Note that it won't compile until the referenced PR is merged upstream so we can use the new versions of
any_spawner
and other in-tree crates.