-
-
Notifications
You must be signed in to change notification settings - Fork 93
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 support for wasm-bindgen #103
base: master
Are you sure you want to change the base?
Conversation
Adds support for num_cpus for wasm32 when used with wasm-bindgen. wasm-bindgen is added as an optional dependency / feature. When used, num_cpus will return result of navigator.hardwareConcurrency.
@seanmonstar Can you take a look at the PR please? I don't really understand the errors on old Rust tbh - it doesn't point to any "special" syntax, but maybe I'm missing something that I'm too used to by now but didn't work on Rust 1.13? |
Co-authored-by: Oliver Wangler <[email protected]>
@seanmonstar Could you please have a look at this PR? Thanks a lot! |
I would also love to see this merged! It would be great to have a cross-platform solution that works on the web. @RReverser, is there any chance that the Edit: reading through your code review, I see that this issue was already discussed. But I'd like to make another push for allowing compilation on all platforms while the feature is active. You can't do platform-specific dependencies in Cargo workspaces, which makes using this crate in workspaces harder. Further, there are a lot of crates - like the |
Why would you enable wasm-bindgen on other platforms? Either way, given how long this PR was open and given that there is now a built-in function for number of CPUs in stdlib, I suspect it will never be merged :( |
I have a cross-platform application in a Cargo workspace. When I add a dependency, I put it under It's not really a big deal. But I thought to raise the issue because this is contrary to how most other crates deal with WASM-specific features. Thanks for the great work on this PR @RReverser! I really do appreciate it, and am using your branch in my project at present 😃 |
Usually libraries want to cut down on dependencies as much as possible to reduce compile-time, which is why it's considered quite unpopular to rely on |
Adds support for num_cpus for wasm32 when used with wasm-bindgen.
When used, num_cpus will return result of navigator.hardwareConcurrency.
This is an alternative to #89 with few notable differences:
web_sys::window()
would fail).f64
(this is an issue in web-sys bindings: web_sys::Navigator::hardware_concurrency should return u64 rustwasm/wasm-bindgen#2425).