Conversation
Also add `wasm32-wasip1` to `targets`. Co-authored-by: BitterGourd <91231822+gaoachao@users.noreply.github.com>
|
📝 WalkthroughWalkthroughRemoved explicit automatic installation of the wasm32 target from CI actions and a composite action, introduced a rust-toolchain.toml specifying channel 1.88.0 and wasm targets, removed a rust-toolchain version pin, adjusted docs, removed some Cargo release profile, and added --enable-bulk-memory to wasm-opt invocations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rust-toolchain.toml (1)
1-4: Optional: tighten profile/components for CI vs. local dev.
- If CI doesn’t run
clippy/rustfmt, considerprofile = "minimal"to cut install size/time. Or keepdefaultbut list components explicitly to decouple from user profile defaults:[toolchain] channel = "1.88.0" targets = ["wasm32-unknown-unknown", "wasm32-wasip1"] -profile = "default" +profile = "default" # or "minimal" in CI if fmt/clippy aren’t required +# components = ["rustfmt", "clippy"]References: rustup profiles. (rust-lang.github.io, ehuss.github.io)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/actions/rustup/action.yml(0 hunks).github/workflows/copilot-setup-steps.yml(0 hunks).github/workflows/deploy-main.yml(0 hunks)CONTRIBUTING.md(1 hunks)rust-toolchain(0 hunks)rust-toolchain.toml(1 hunks)
💤 Files with no reviewable changes (4)
- .github/workflows/deploy-main.yml
- .github/actions/rustup/action.yml
- rust-toolchain
- .github/workflows/copilot-setup-steps.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: copilot-setup-steps
🔇 Additional comments (2)
rust-toolchain.toml (2)
1-4: Good pin to 1.88.0; correct WASI target name.
- Bumping to 1.88.0 and listing
wasm32-wasip1(the rename ofwasm32-wasi) is correct for 2025; removal of the oldwasm32-wasion stable happened in 1.84 (Jan 9, 2025).
References: Rust blog on WASI target rename and schedule; rustc platform docs. (blog.rust-lang.org, doc.rust-lang.org)
1-4: No legacy rust-toolchain file found
Verified that no plainrust-toolchainoverride file exists in the repository.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Web Explorer#5044 Bundle Size — 361.93KiB (-1.37%).db3fbcf(current) vs 67ee3e6 main#5015(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch colinaaa:colin/0910/rust-188 Project dashboard Generated by RelativeCI Documentation Report issue |
React Example#5051 Bundle Size — 238.2KiB (0%).db3fbcf(current) vs 67ee3e6 main#5022(baseline) Bundle metrics
|
| Current #5051 |
Baseline #5022 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
163 |
163 |
|
67 |
67 |
|
46.88% |
46.88% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #5051 |
Baseline #5022 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
92.45KiB |
92.45KiB |
Bundle analysis report Branch colinaaa:colin/0910/rust-188 Project dashboard
Generated by RelativeCI Documentation Report issue
CodSpeed Performance ReportMerging #1712 will improve performances by 12.61%Comparing Summary
Benchmarks breakdown
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/web-platform/web-style-transformer/scripts/build.js (2)
45-47: Double-check “legacy” artifact’s feature budget.If “legacy” must remain MVP-only, allowing bulk-memory here could enable Binaryen to introduce or preserve bulk-memory ops, raising the required runtime features. If legacy compatibility isn’t a concern anymore, keep as-is; otherwise gate this flag for legacy builds.
Example tweak (only if you need to keep legacy MVP-only):
-execSync( - `pnpm wasm-opt --enable-bulk-memory ./dist/legacy_bg.wasm -O3 -o ./dist/legacy_bg.wasm`, - { cwd: packageRoot, stdio: 'inherit' }, -); +execSync( + `pnpm wasm-opt ${process.env.LEGACY_ALLOW_BULK ? '--enable-bulk-memory ' : ''}./dist/legacy_bg.wasm -O3 -o ./dist/legacy_bg.wasm`, + { cwd: packageRoot, stdio: 'inherit' }, +);Then set LEGACY_ALLOW_BULK=true in environments where legacy can require that feature.
27-29: Deduplicate wasm-opt invocations.Minor clean-up to avoid drift between standard/legacy flags.
Apply within these hunks:
-execSync( - `pnpm wasm-opt --enable-bulk-memory ./dist/standard_bg.wasm -O3 -o ./dist/standard_bg.wasm`, - { cwd: packageRoot, stdio: 'inherit' }, -); +optimizeWasm('standard', '--enable-bulk-memory'); ... -execSync( - `pnpm wasm-opt --enable-bulk-memory ./dist/legacy_bg.wasm -O3 -o ./dist/legacy_bg.wasm`, - { cwd: packageRoot, stdio: 'inherit' }, -); +optimizeWasm('legacy', '--enable-bulk-memory');Add this helper near the top of the file:
function optimizeWasm(kind, extraFlags = '') { const file = `./dist/${kind}_bg.wasm`; execSync( `pnpm wasm-opt ${extraFlags} ${file} -O3 -o ${file}`, { cwd: packageRoot, stdio: 'inherit' }, ); }Also applies to: 45-47
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/web-platform/web-style-transformer/Cargo.toml(0 hunks)packages/web-platform/web-style-transformer/scripts/build.js(2 hunks)
💤 Files with no reviewable changes (1)
- packages/web-platform/web-style-transformer/Cargo.toml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T12:38:10.439Z
Learnt from: CR
PR: lynx-family/lynx-stack#0
File: AGENTS.md:0-0
Timestamp: 2025-09-09T12:38:10.439Z
Learning: Always run a full build (pnpm turbo build) before running any tests
Applied to files:
packages/web-platform/web-style-transformer/scripts/build.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: test-rust / clippy
- GitHub Check: copilot-setup-steps
🔇 Additional comments (1)
packages/web-platform/web-style-transformer/scripts/build.js (1)
27-29: Verify wasm-opt bulk-memory support
Confirm that the Binaryen v124.0.0wasm-optbinary installed inpackages/web-platform/web-style-transformerrecognizes the--enable-bulk-memoryflag required for Rust 1.88 WASM optimizations (e.g. runpnpm wasm-opt --help | grep -- '--enable-bulk-memory'in that package).
This is cherry-picked from #1458.
Summary by CodeRabbit
Chores
Documentation
Checklist