feat: enable wasm32 compilation by making xx crate conditional#570
feat: enable wasm32 compilation by making xx crate conditional#570
xx crate conditional#570Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #570 +/- ##
==========================================
- Coverage 79.04% 73.56% -5.49%
==========================================
Files 48 48
Lines 7235 7361 +126
Branches 7235 7361 +126
==========================================
- Hits 5719 5415 -304
- Misses 1141 1255 +114
- Partials 375 691 +316 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enables wasm32 compatibility by making the xx dependency conditional and replacing its utility functions with standard library and regex crate equivalents. The review feedback highlights a performance regression where regexes previously compiled at compile-time (via the xx::regex! macro) are now being recompiled on every function call. It is recommended to use std::sync::LazyLock to define these regexes as static variables to restore efficient execution.
cli/src/cli/complete_word.rs
Outdated
| let stdout = sh(&run)?; | ||
| // trace!("stdout: {stdout}"); | ||
| let re = regex!(r"[^\\]:"); | ||
| let re = Regex::new(r"[^\\]:").unwrap(); |
There was a problem hiding this comment.
Compiling this regex on every call to complete_arg is inefficient. The original xx::regex! macro compiled it at compile-time. To retain similar performance, you should compile this regex only once. You can use std::sync::LazyLock (which is already imported) to define it as a static variable at the module level.
For example:
static COMPLETION_DESC_SPLIT_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"[^\\]:").unwrap());Then, you can use it here like this:
let re = &*COMPLETION_DESC_SPLIT_RE;
// or directly
match COMPLETION_DESC_SPLIT_RE.find(l) //...
lib/src/docs/markdown/renderer.rs
Outdated
| } | ||
| // replace '<' with '<' but not inside code blocks | ||
| xx::regex!(r"(`[^`]*`)|(<)") | ||
| Regex::new(r"(`[^`]*`)|(<)").unwrap() |
There was a problem hiding this comment.
Compiling this regex for every line being processed is inefficient. The original xx::regex! macro avoided this by compiling at compile-time. To maintain performance, you should compile this regex only once. You can use std::sync::LazyLock to define it as a static variable at the module level.
For example:
use std::sync::LazyLock;
use regex::Regex;
static ESCAPE_MD_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"(`[^`]*`)|(<)").unwrap());Then you can use it here like ESCAPE_MD_RE.replace_all(...).
lib/src/docs/markdown/renderer.rs
Outdated
| let path_re = | ||
| regex!(r"https://(github.com/[^/]+/[^/]+|gitlab.com/[^/]+/[^/]+/-)/blob/[^/]+/"); | ||
| tera.register_function("source_code_link", |args: &HashMap<String, tera::Value>| { | ||
| let path_re = Regex::new(r"https://(github.com/[^/]+/[^/]+|gitlab.com/[^/]+/[^/]+/-)/blob/[^/]+/").unwrap(); |
There was a problem hiding this comment.
This regex is compiled every time render is called. Since it's a constant pattern, it's more efficient to compile it once using std::sync::LazyLock and reuse the compiled Regex object across calls. This avoids the performance overhead of repeated regex compilation.
For example, at the module level:
use std::sync::LazyLock;
use regex::Regex;
static SOURCE_CODE_PATH_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"https://(github.com/[^/]+/[^/]+|gitlab.com/[^/]+/[^/]+/-)/blob/[^/]+/").unwrap());Then you can use SOURCE_CODE_PATH_RE directly in your register_function closure.
lib/src/spec/mod.rs
Outdated
| if full.starts_with("#!") { | ||
| let usage_regex = xx::regex!(r"^(?:#|//|::)(?:USAGE| ?\[USAGE\])"); | ||
| if full.lines().any(|l| usage_regex.is_match(l)) { | ||
| let usage_re = Regex::new(r"^(?:#|//|::)(?:USAGE| ?\[USAGE\])").unwrap(); |
There was a problem hiding this comment.
This regex is compiled on every call to split_script. It's more efficient to compile it once using std::sync::LazyLock and reuse it. The original xx::regex! macro handled this by compiling at compile-time.
For example, at the module level:
use std::sync::LazyLock;
use regex::Regex;
static USAGE_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"^(?:#|//|::)(?:USAGE| ?\\[USAGE\\])").unwrap());Then you can use USAGE_RE here.
lib/src/spec/mod.rs
Outdated
| let usage_regex = Regex::new(r"^(?:#|//|::)(?:USAGE| ?\[USAGE\])(.*)$").unwrap(); | ||
| let blank_comment_regex = Regex::new(r"^(?:#|//|::)\s*$").unwrap(); |
There was a problem hiding this comment.
These regexes are compiled on every call to extract_usage_from_comments. For better performance, they should be compiled only once using std::sync::LazyLock and stored in static variables, similar to how xx::regex! worked. This will avoid performance degradation, especially since this function processes file contents line by line.
Greptile SummaryThis PR enables Key changes:
Confidence Score: 5/5Safe to merge — all previous P1 concerns (missing The changes are structurally sound: cfg guards are consistent across
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[compile target] -->|wasm32| B[xx excluded\nCargo.toml cfg guard]
A -->|non-wasm32| C[xx included\nnormal path]
B --> D[sh stub\nreturns Unsupported error]
B --> E[std::fs::read_to_string\nfor file reads]
B --> F[LazyLock<Regex>\nfor pattern matching]
B --> G[std::fs::create_dir_all\n+ std::fs::write]
C --> H[xx::process::check_status\nfor sh execution]
C --> I[std::fs::read_to_string\nfor file reads]
C --> J[LazyLock<Regex>\nfor pattern matching]
C --> K[std::fs::create_dir_all\n+ std::fs::write]
D --> L[UsageErr::IO Unsupported\nor miette error]
H --> M[XXResult / miette error]
Reviews (2): Last reviewed commit: "[autofix.ci] apply automated fixes" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Make the xx crate dependency conditional on non-wasm32 targets and replace xx::regex!, xx::file, and xx::process usages with std equivalents (LazyLock<Regex>, std::fs, std::process) so the codebase compiles for wasm32-wasip1. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
andreaTP
force-pushed
the
java-wasm-enablement
branch
from
e23f7b7 to
3e7327c
Compare
|
All comments should be addressed. |
1 participant
Supersedes #561 this PR contains only the Rust-level changes needed for wasm32 support to avoid having to patch on top downstream.
The
xxcrate doesn't support wasm32. This makesxxa conditional dependency (cfg(not(target_arch = "wasm32"))) and replaces its usages (xx::regex!,xx::file,xx::process) with std equivalents.shexecution is disabled on wasm (returns an error).CI now includes a minimal
cargo build --target wasm32-wasip1check to prevent regressions and this is all I need to bundle and ship ausage4jlibrary usable and embeddable in Java.Please @jdx have a look and let me know your thoughts 🙏