Conversation
|
|
There was a problem hiding this comment.
I had to move this project to be a Cargo workspace so that it can host the Rust+WASM projects in the same repo
There was a problem hiding this comment.
It should work to have the rust code still in the root directory and that would be preferable. There's an example in https://github.com/denoland/deno_graph/blob/a63963643a56fb664c980149bc20d17484d9513c/Cargo.toml#L13
| @@ -1,3 +1,3 @@ | |||
| [toolchain] | |||
| channel = "1.76.0" | |||
| channel = "1.78.0" | |||
There was a problem hiding this comment.
cargo make only supports >1.78.0. Although there are more recent Rust versions, I wanted to make the minimum version bump required in this PR to play it safe
| description = "Build WASM target for deno_task_shell" | ||
| command = "wasm-pack" | ||
| install_crate = { crate_name = "wasm-pack", binary = "wasm-pack", test_arg = "-V" } | ||
| args = ["build", "--target", "deno", "--scope", "@deno/deno_task_shell"] |
There was a problem hiding this comment.
One open question is what the name of this WASM question should be. I think the name @deno/deno_task_shell is okay, but in reality we're only exposing the task parsing API so maybe it should be something that better conveys the scope like @deno/shell-cmd-processing
There was a problem hiding this comment.
I took this file as-is from dax. I'm not sure if there is anything in this file that should be changed, so I left it as-is to start
There was a problem hiding this comment.
We should remove console_static_text and once_cell.
| @@ -0,0 +1,5 @@ | |||
| [tasks.wasm-build] | |||
| description = "Build WASM target for deno_task_shell" | |||
| command = "wasm-pack" | |||
There was a problem hiding this comment.
Although my PR properly generates a WASM file, it's not clear to me what the best way to publish it would be given the new WASM support in Deno 2.1
I created an issue on JSR & wasm-pack to try and clarify this: drager/wasm-pack#1454
dsherret
left a comment
dsherret
left a comment
There was a problem hiding this comment.
Probably we should pattern off https://github.com/denoland/deno_graph/tree/main for this. We use https://github.com/denoland/wasmbuild and not wasmpack.
Also, for the name, maybe @deno/task-shell-parser or something like that?
| CARGO_MAKE_EXTEND_WORKSPACE_MAKEFILE = true | ||
|
|
||
| [tasks.wasm-build] | ||
| # see wasm/Makefile.toml for task definition |
There was a problem hiding this comment.
Can you remove this file and instead use a deno.json with tasks? We'd rather not have to add cargo install cargo-make to the setup instructions.
There was a problem hiding this comment.
I tried wasmbuild but it only generates a single JS file that gets instantiated which felt like the old way to generate things given wasm-bindgen/wasm-bindgen#4287
I'm not sure if the plan is to update wasmbuild in some way, but I believe this is the issue tracking this: denoland/wasmbuild#145
| ).await; | ||
| ``` | ||
|
|
||
| ## WASM |
There was a problem hiding this comment.
| ## WASM | |
| ## Wasm |
There was a problem hiding this comment.
We should remove console_static_text and once_cell.
3 participants
Closes #136