feat: publish xlsx import export as separate wasm#381
Conversation
BrianHung
changed the title
BrianHung
force-pushed
the
import-export-xlsx-icalc-separate-wasm
branch
from
3fe119d to
b9f70b0
Compare
|
Hi @BrianHung, This is definitely a good idea! And people have asked for this. Lets do it. But I am not sure if we can use the xlsx crate as is, as that crate might grow in considerable size. That being said my first problem is that building the wasm as per your instructions I actually get a smaller build! What am I doing wrong? |
| zip = "0.6" | ||
| zip = { version = "0.6.6", default-features = false, features = ["deflate", "time"] } | ||
| flate2 = { version = "1.0", default-features = false, features = ["rust_backend"] } | ||
| time = { version = "0.3", features = ["wasm-bindgen"] } |
There was a problem hiding this comment.
Could you explain why we need all this?
There was a problem hiding this comment.
xlsx are actually zip files, so we need to bundle the utility functions flate2 (for deflating a zip file) and time (timestamps in headers) along with it.
|
Or is it that you need both wasm files? That is an awesome feat! I was imagining you would have two builds:
I have done that in the past and discarded for now as I didn't see an immediate benefit for IronCalc. But having two different packages is interesting as you could load the second one on demand only when importing/exporting. |
|
|
||
| fn main() { | ||
| // This is required for cargo to compile this as a binary | ||
| } |
There was a problem hiding this comment.
I'm a bit curious about this, why didn't you go the same way as the other build? Note in Cargo you can have different build targets. See for instance in IronCalc:
https://github.com/ironcalc/IronCalc/blob/main/base/Cargo.toml#L24-L27
https://github.com/ironcalc/IronCalc/blob/main/base/src/model.rs#L38-L64
Maybe that would make things easier? We just have two builds with different features?
There was a problem hiding this comment.
This is so we can have one crate, but two wasm files outputted.
If there isn't a main declaration, cargo would just treat it as a library and won't turn it into an artifact.
I'm pretty new to rust, so if you have a preferred method that achieves the same goal, I would defer to you here!
nhatcher
left a comment
nhatcher
left a comment
There was a problem hiding this comment.
Hi @BrianHung, I left a few questions, mostly for my own understanding. Once I understand what is going on I would like to streamline the process a bit. But I think this is awesome!
Yep, that's right! We keep ironcalc and xlsx import/export as separate wasm files so we don't cause client to have an unnecessary larger bundle size if they don't use xlsx utilities. |
Resolved conflicts: - .github/workflows/rust-build-test.yaml: accepted deletion (replaced by build-test.yaml in main) - Cargo.lock: accepted main's version - Makefile: kept xlsx_wasm build steps, added test-python target from main - bindings/wasm/Cargo.toml: kept ironcalc (xlsx) dependency, updated versions to 0.7 - bindings/wasm/src/lib.rs: kept js_name attribute, added language_id parameter
- load_from_icalc_bytes now requires language_id - fromXLSXBytes and toXLSXBytes now require language_id - Update tests to pass language_id argument - Update Model constructors to use 4 arguments
- Add lifetime parameter to load_from_icalc_bytes function - Fix code formatting in xlsx.rs
- Add wasm32-unknown-unknown target to lint-all and test-js jobs - Install wasm-bindgen-cli for test-js job (needed for xlsx_wasm build)
|
Bumping this to ask if there is anything blocking progress here, or is this on hold for a specific reason? Just trying to understand the status. For context, I came across this PR while looking for a way to handle ironcalc's xlsx import/export in WASM, which is something I'd find useful. |
|
We'd also be interested in a making it possible to convert xlsx from the browser (see #1113). Having a separate wasm to load for that feature is fine. |
4 participants
Separates xlsx into its own wasm file to address bundle size increase concerns in #379.
Had to update wasm-bindgen to avoid a separate crate just for xlsx.wasm.