Wasm feature gate - Githubissues

bokuweb / docx-rs

:memo: A .docx file writer with Rust/WebAssembly.

https://bokuweb.github.io/docx-rs/

MIT License

345 stars 62 forks source link

Wasm feature gate #622

Closed arifd closed 1 year ago

arifd commented 1 year ago

What does this change?

Puts wasm related code behind a feature gate so that users who do not need this feature do not need to compile it

References

I didn't take a note of the first search/replace but here are the others that i did:

all within docx-core/

search
#\[derive\((.*) ts_rs::TS\)\]

replace
#[derive($1)]\n#[cfg_attr(feature = "wasm", derive(ts_rs::TS))]

--

search
#[ts(export)]

replace
#[cfg_attr(feature = "wasm", ts(export))]

Misc

THIS PR CURRENTLY BREAKS TESTS. please advise, I'm not sure what I have done wrong.
I did not put serde_json behind the feature flag, not sure if it is used in the case of wasm only.
Users who pull in docx-rs with default-features = false AND rely on wasm, this will be a breaking change for them.

bokuweb commented 1 year ago

@arifd Thanks!!

arifd commented 1 year ago

@bokuweb, did you read my notes? Tests are broken. Will you take a look?

bokuweb commented 1 year ago

@arifd oops. I'll check it later.

arifd commented 1 year ago

Also, because the toolchain is stuck on 1.58, the rust edition is stuck on 2018, which means to expose the wasm feature, we also implicitly expose features "wasm-bindgen" and "ts-rs". there's no way around it pre edition 2021.

If you allow me to remove the toolchain restraint (which appears to be added for a reason that is very unclear), i'm happy to bump the edition, and hide the two additional features

arifd commented 1 year ago

Oh, I haven't confirmed it, but most likely the tests now just need to have the "wasm" feature applied! silly me!

bokuweb commented 1 year ago

@arifd

did you read my notes? Tests are broken. Will you take a look?

Currently the test seems to pass both CI and local. https://github.com/bokuweb/docx-rs/pull/623

Also, because the toolchain is stuck on 1.58, the rust edition is stuck on 2018, which means to expose the wasm feature, we also implicitly expose features "wasm-bindgen" and "ts-rs". there's no way around it pre edition 2021.

PR welcome :)