Skip to content

Conversation

@kdy1
Copy link
Member

@kdy1 kdy1 commented Oct 28, 2025

Description:

Related issue (if exists):

@kdy1 kdy1 added this to the Planned milestone Oct 28, 2025
@changeset-bot
Copy link

changeset-bot bot commented Oct 28, 2025

⚠️ No Changeset found

Latest commit: e9e2c97

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@claude
Copy link
Contributor

claude bot commented Oct 28, 2025

Claude finished @kdy1's task —— View job


PR Review: Expose wasmtime backend

I've completed my review of this pull request. Here's my comprehensive feedback:

Review Progress:

  • Read and analyze modified files
  • Review Cargo.toml dependency changes
  • Check feature flag usage and organization
  • Assess security implications of exposing wasmtime backend
  • Evaluate performance considerations
  • Check test coverage requirements
  • Provide comprehensive feedback

Code Quality and Best Practices

Strengths:

  • Clean, consistent feature flag naming following existing patterns (plugin_backend_wasmtime matches plugin_backend_wasmer)
  • Proper use of optional dependencies with default-features = false
  • Consistent code formatting and organization
  • Good feature flag organization in both swc and swc_core crates

Minor improvements:

  • The dependency reordering in crates/swc/Cargo.toml:120-121 moves swc_plugin_backend_wasmtime before swc_plugin_proxy - this is fine for alphabetical ordering but consider documenting the reasoning for dependency ordering if there's a specific pattern

Feature Flag Implementation

Well implemented:

  • Feature flag correctly delegates to the underlying swc crate: plugin_backend_wasmtime = ["swc/plugin_backend_wasmtime"]
  • Follows the same pattern as plugin_backend_wasmer
  • Proper optional dependency management

Note: The change to make wasmer dependencies optional with ? syntax (swc_plugin_backend_wasmer?/plugin_transform_host_js) is a good improvement that makes the feature flags more flexible.

Security Considerations

Low risk changes:

  • This PR only exposes existing functionality through feature flags
  • The wasmtime backend itself (swc_plugin_backend_wasmtime) already exists and is tested
  • No new attack surface is introduced - only making existing functionality optionally available
  • Uses default-features = false which is a security best practice

Performance Considerations

Positive aspects:

  • Wasmtime is a production-ready WebAssembly runtime with good performance characteristics
  • Optional feature flags mean no performance impact unless explicitly enabled
  • Provides users with choice between wasmer and wasmtime backends based on their performance needs

Recommendation: Consider adding documentation about when to choose wasmtime vs wasmer backends, as users will now have this choice.

Test Coverage

Good coverage:

  • The wasmtime backend is already well-tested in crates/swc_plugin_backend_tests/
  • Tests exist for both ecma and css transforms with wasmtime: crates/swc_plugin_backend_tests/tests/ecma_rkyv.rs:225
  • Integration tests and benchmarks are present

⚠️ Missing Changeset

The changeset-bot correctly identified that this change needs a changeset since it exposes new public API surface. This should result in at least a patch version bump for swc_core.

📝 Recommendations

  1. Add a changeset as noted by the changeset-bot - this is a new feature that should be documented in release notes

  2. Consider documentation: Add a brief note in relevant documentation about the choice between wasmtime and wasmer backends, including performance characteristics or use case recommendations

  3. Verify CLI/binding integration: Ensure that higher-level bindings (Node.js, WASM, CLI) can properly utilize this new backend option if needed

🎯 Overall Assessment

This is a clean, well-implemented PR that follows existing patterns and best practices. The changes are minimal, focused, and low-risk. The code quality is high and the feature flag implementation is consistent with the existing codebase architecture.

Recommendation: ✅ Approve after adding the changeset

The only blocking issue is the missing changeset, which should be added to properly version and document this new feature.


@github-actions
Copy link
Contributor

Binary Sizes

File Size
swc.linux-x64-gnu.node 36M (37113184 bytes)

Commit: b14ba87

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 28, 2025

CodSpeed Performance Report

Merging #11189 will not alter performance

Comparing kdy1/wasmtime (e9e2c97) with main (037131c)1

Summary

✅ 138 untouched
⏩ 13 skipped2

Footnotes

  1. No successful run was found on main (358a508) during the generation of this report, so 037131c was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

  2. 13 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants