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

migrate WASM example and documentation+Wiki from webpack to trunk #834

Closed
spookyvision opened this issue Feb 9, 2024 · 3 comments · Fixed by #844
Closed

migrate WASM example and documentation+Wiki from webpack to trunk #834

spookyvision opened this issue Feb 9, 2024 · 3 comments · Fixed by #844

Comments

@spookyvision
Copy link
Contributor

Out of curiosity, I tried migrating the WASM example to trunk and it worked out of the box. In my opinion this is much more straightforward (does not require any JavaScript nor npm and a big node_modules folder). Are you interested in this change? If yes, I'll create a PR.

@spookyvision
Copy link
Contributor Author

spookyvision commented Feb 9, 2024

another alternative would be cargo-run-wasm, with which the example could be entirely self-contained (no external dependencies, at the cost of a slightly more complicated directory structure and a few additional lines of code)

@est31
Copy link
Member

est31 commented Feb 10, 2024

Yeah I agree, trunk is way better than webpack for the reasons you stated.

@spookyvision
Copy link
Contributor Author

spookyvision commented Feb 13, 2024

I thought about it some more and I'd say trunk is preferable over cargo-run-wasm, since the latter is really only suitable for tiny examples (can't customize the HTML at all, except by manipulating the DOM, which ... meh). I'll go ahead and send a trunk-based PR soon.

est31 pushed a commit that referenced this issue Feb 25, 2024
## Description

This PR migrates the wasm example from an npm/webpack based approach to [trunk](https://trunkrs.dev/). Fixes #834.

Of note:
the `console_error_panic_hook` dependency was gated in `Cargo.toml` via `[target."cfg(debug_assertions)".dependencies]`. This is documented as nonfunctional and will throw a warning on build:
```
warning: Found `debug_assertions` in `target.'cfg(...)'.dependencies`. 
This value is not supported for selecting dependencies and will not work as expected. 
To learn more visit https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#platform-specific-dependencies
```

Since the actual usage of `console_error_panic_hook` is *also* gated in `lib.rs` behind `#[cfg(debug_assertions)]` it didn't make sense to have the cargo gate in place, hence I have removed it.

## Next steps

If this PR is merged, the [wiki page](https://github.com/RustAudio/cpal/wiki/Setting-up-a-new-CPAL-WASM-project) linked in the root README should be updated as well.
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.

2 participants