-
Notifications
You must be signed in to change notification settings - Fork 39
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
add Zig support #144 #196
add Zig support #144 #196
Conversation
I saw you already included the documentation! That's amazing @voigt 😄 |
Amazing contribution, thanks @voigt! Although I don't have experience with Zig, I expect to be able to have a look at this PR this week :) |
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.
Thanks for the amazing and important contribution @voigt! I think this is already in a very nice shape :)
Disclaimer: despite I like Zig's ideas I don't have experience with it, so I cannot help that much on Zig's specifics. :)
Some comments from my side on a first pass. Thanks again! 👏
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.
This is an amazing contribution @voigt! I tested the different examples in my laptop and I could run my first Zig workers!!
$ wws zig-out/bin/
⚙️ Preparing the project from: zig-out/bin/
⚙️ Loading routes from: zig-out/bin/
⏳ Loading workers from 1 routes...
✅ Workers loaded in 46.583667ms.
- http://127.0.0.1:8080/basic
=> zig-out/bin/basic.wasm
🚀 Start serving requests at http://127.0.0.1:8080
[2023-08-28T07:58:55Z INFO actix_web::middleware::logger] 127.0.0.1 "GET /basic HTTP/1.1" 200 682 "-" "..." 0.003404
As you mentioned, environment variables are not working. It would be great to dig into this issue to understand if this is a Zig bug. We can create a separate issue for it, but for now I would delete the zig-envs
example from the PR.
I added some minor comments. For me, the only important topic to clarify is the input
read process and \n
.
Again thank you! I'm really glad to see Zig support on wws
😄
Thank you for your feedback @Angelmmiguel & @ereslibre! I will incorporate it asap, during the course of this week :) |
Co-authored-by: Rafael Fernández López <ereslibre@gmail.com>
Co-authored-by: Rafael Fernández López <ereslibre@gmail.com>
Co-authored-by: Rafael Fernández López <ereslibre@gmail.com>
8e38e68
to
52e0940
Compare
This reverts commit 4a942f2.
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.
Amazing contribution @voigt! I'm really happy to see Zig on Wasm Workers Server! We definitely should write an article about it 😉
I added a last minor comment on the zig-basic
example.
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.
Huge LGTM, thanks @voigt for your efforts!
Tried all the examples and they worked perfectly 🎉
This is really cool!! 👏 👏 Thank you very much for your contribution @voigt! Related CLA: #210 (comment) |
This PR adds a Zig library to write Workers for wasm-worker-server in Zig (#144) .
Fixes: #144
Some remarks:
Request
andResponse
from Zigs standard library. Unfortunatelystd.http.Server.Response
as well asstd.http.Client.Request
are not sufficient :( Usingstd.http.Server.Response
even lead to a WASI compile errorerror: root struct of file 'os.wasi' has no member named 'sockaddr'
. Therefore I created custom entities and hope that we can fix this in the futurestd.os.getenv()
does not work properly/differently than expected.Work to be done before merge: