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

Make WASM compilation an optional feature #26

Closed
alerque opened this issue Jul 10, 2024 · 4 comments · Fixed by #27
Closed

Make WASM compilation an optional feature #26

alerque opened this issue Jul 10, 2024 · 4 comments · Fixed by #27

Comments

@alerque
Copy link
Contributor

alerque commented Jul 10, 2024

I'm having some trouble compiling my decasify project to WASM. This is my first real go at setting up a WASM build, so I might be doing it wrong, but it seems the trouble is the titlecase() function I'm trying to export (decasify's version handling multiple locales not just English like this project's function of the same name) is a naming conflict with the one in this crate. It doesn't even matter what I name my function on the Rust side or that I'm importing this crate's function with an alias — even if I try to take some other unrelated function and send it to the JS side with a #[wasm_bindgen(js_name = titlecase)] attribute macro it ends up producing a duplicate symbol that the linker can't work out.

As best I can figure the issue is with this:

[target.'cfg(target_family = "wasm")'.dependencies]

... and the related automatic export of a WASM function with the name titlecase even if my app doesn't import that function. Just the fact that my app is being target at WASM creates the conflict.

This project seems to be unconditionally adding #[wasm_bindgen] attributes to functions as well if the compilation target is WASM. Instead of this would it be possible to add an optional feature to gate this so that it would be possible to inclnude this crate and link against it using rlib without wasm exports getting in the way?

@wezm
Copy link
Owner

wezm commented Jul 10, 2024

Hmm that is an interesting problem that I didn't foresee. Thanks for working on a fix. Since the exported function doesn't do anything other than call crate::titlecase I might see if there's a way to avoid the need for the wasm module entirely.

@alerque
Copy link
Contributor Author

alerque commented Jul 10, 2024

Yes, it's a bit of an unexpected issue, and seemily a bit of a Rust complier limitation. I'm even importing your function with an alias:

use titlecase::titlecase as gruber_titlecase;

But the mere existence of the titlecase() function targeting WASM in this crate is blocking by ability to export a function of the same name.

I don't think dropping thet wasm module would help; even if you moved that up to the main library you'd still be exporting a titlecase symbol any time the Rust compiler is targeting WASM even if it wasn't being asked for or explicitly used.

@wezm
Copy link
Owner

wezm commented Jul 10, 2024

I don't think dropping they wasm module would help

I expect it would be fine with the wasm module removed (and thus the call to the wasm_bindgen macro. The #[wasm_bindgen] macro is exporting a top-level symbol, without it the the normal name mangling should ensure there's no conflicts.

@alerque
Copy link
Contributor Author

alerque commented Jul 11, 2024

Perhaps I'm not understanding what you are proposing. Whether the Rust code for it is laid out in a module or inlined into the top level lib or wherever you put it, at the end of the day you want this crate to be compilable to WASM. That's totally fine, but one way or another that means getting a wasm_bindgen attribute in there somewhere. Unless you split it into two crates, the only way to accomplish your goal without blocking mine is to gate it somehow. Currently your gate mechanism is just the target architecture. I'm saying that trips me up because my target architecture is WASM too, but I don't want to trigger this crate building it's own WASM interface, I only want to build my crate's WASM interface. Hence the need for a different gate mechanism.

@wezm wezm closed this as completed in #27 Jul 12, 2024
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