-
Notifications
You must be signed in to change notification settings - Fork 35
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
Remove wasm-bindgen deps from rust #343
Conversation
Rust CML crates must use wasm-bindgen for enums as these are used from the WASM CML crates. Using noop_proc_macro we can replace the wasm_bindgen proc macro with a no-op that does nothing when consumed directly from rust. This is now locked behind a feature (used_from_wasm) as we can't just use cfg target checks as compiling the wasm crate (not using web-pack) would use the rust target and break. Crates using cml-chain and cml-multi-era from a wasm crate must specify this feature to activate the wasm_bindgen dependancy instead of the no-op macro.
|
||
# only for declaring hash types | ||
bech32 = "0.7.2" | ||
hex = "0.4.0" | ||
|
||
# non-wasm | ||
noop_proc_macro = { version = "0.3.0" } |
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.
I had this as optional = true
but for some reason when consuming it from dcspark-core/carp it would not be able to find the dependancy. Strangely having it as optional worked fine for the cml-chain
dependency. Both were specified in the same way in the same Cargo.toml
file in dcspark-core so I have no idea why this is the case.
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.
doesn't cml-chain
have it as optional = false
? which would be the same as not writing it
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.
Oops, I saw it specified and I guess my brain just read it as = true
automatically. That makes sense now.
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.
LGTM.
Maybe the feature could be called just wasm
or with_wasm_enums
or something like that? But I think it's fine anyways.
Rust CML crates must use wasm-bindgen for enums as these are used from the WASM CML crates. Using noop_proc_macro we can replace the wasm_bindgen proc macro with a no-op that does nothing when consumed directly from rust. This is now locked behind a feature (used_from_wasm) as we can't just use cfg target checks as compiling the wasm crate (not using web-pack) would use the rust target and break. Crates using cml-chain and cml-multi-era from a wasm crate must specify this feature to activate the wasm_bindgen dependancy instead of the no-op macro.
Rust CML crates must use wasm-bindgen for enums as these are used from the WASM CML crates.
Using noop_proc_macro we can replace the wasm_bindgen proc macro with a no-op that does nothing when consumed directly from rust.
This is now locked behind a feature (used_from_wasm) as we can't just use cfg target checks as compiling the wasm crate (not using web-pack) would use the rust target and break.
Crates using cml-chain and cml-multi-era from a wasm crate must specify this feature to activate the wasm_bindgen dependancy instead of the no-op macro.