-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 redux #1190
Wasm redux #1190
Conversation
@stevepentland, thanks for your PR! By analyzing the history of the files in this pull request, we identified @kbknapp to be a potential reviewer. |
Sorry it took me so long, since we can't really run CI against this, I wanted to set up a local wasm test to ensure it all works. Turns out there are still quite a few things that need to happen, as it wouldn't build just yet. I was able to get it building, and have the changes locally, but there may be a better way so I'll just tell you roughly what I did in case you have more ideas.
I'm assuming we should also not compile |
@kbknapp It builds without any problem against wasm32-unknown-unknown. Were you trying against emscripten?
Edit 2: The original target linked in #1132 bug is also wasm32-unknown-unknown. In the README changes I documented only compatibility with this particular target, omitting emscripten intentionally. |
Ah yes, I should have mentioned I built against both So also, feel free to correct me because I'm not a web/wasm guy at all but is there no advantage to building for emscripten and |
👍 gotcha I'm good with this |
emscripten makes for much larger code and is slower from what I understand as it was using some interpreter. The newer unknown-unknown is the rust 'native' one and I believe is what we're going ahead with in rust core. |
Ok, sounds good, thanks for explaining and for all the work on this! |
No problem! And thanks for helping me figure out more about clap! It was really helpful to get your input. |
This is a complete rebase of the existing changes in #1187 to clean up the history
cc: @kbknapp
This change is